[GitHub] flink pull request: [FLINK-3758] Add possibility to register accum...
Github user aljoscha commented on the pull request: https://github.com/apache/flink/pull/1979#issuecomment-218820979 No one should implement it. With interfaces it's hard to now what the stance is, that's why I'm including those two. @rmetzger implemented the tool that will check whether releases break API compatibility and @StephanEwen always has an eye on these. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-3758] Add possibility to register accum...
Github user knaufk commented on the pull request: https://github.com/apache/flink/pull/1979#issuecomment-218819781 I was worried about this to and intially only added the method to the AbstractRuntimeUDFContext. On the other hand, are you aware of anyone who has implemented RuntimeContext? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-3758] Add possibility to register accum...
Github user aljoscha commented on the pull request: https://github.com/apache/flink/pull/1979#issuecomment-218817622 R: @rmetzger R: @StephanEwen What do you think about the interface. This adds a method to `RuntimeContext`. This does not break if users are only using it. It would break if someone implements it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-3758] Add possibility to register accum...
Github user aljoscha commented on the pull request: https://github.com/apache/flink/pull/1979#issuecomment-218816900 Ok, thanks! I'm merging it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-3758] Add possibility to register accum...
Github user knaufk commented on the pull request: https://github.com/apache/flink/pull/1979#issuecomment-218815109 Hopefully, fixed the formatting stuff. If there are still missing lines or so, please just go ahead and fix it during merging... --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-3758] Add possibility to register accum...
Github user aljoscha commented on the pull request: https://github.com/apache/flink/pull/1979#issuecomment-218805483 Nice and straightforward, I just had some minor comments about formatting. ð And of course we protect the feature with a test. If you want I can fix the formatting while merging. Or you can fix it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-3758] Add possibility to register accum...
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/1979#discussion_r63047675
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/windowing/triggers/Trigger.java
---
@@ -169,7 +170,17 @@ public void clear(W window, TriggerContext ctx) throws
Exception {}
* function (function is
not part os a KeyedStream).
*/
S getPartitionedState(StateDescriptor
stateDescriptor);
-
+
+ /**
+*
+* Get an existing accumulator by name if it
exists, Otherwise add defaultAccumulator.
+*
+* @param name Name of the accumulator
+* @param defaultAccumulator Accumulator to add, if no
accumulator of the given name exists
+* @return The accumulator registered by the given
name or the given default defaultAccumulator
+*/
+ public Accumulator
getAccumulator(String name,
org.apache.flink.api.common.accumulators.Accumulator defaultAccumulator);
--- End diff --
`org.apache.flink.api.common.accumulators.Accumulator` can be shortened to
`Accumulator`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3758] Add possibility to register accum...
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/1979#discussion_r63047584
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/windowing/triggers/Trigger.java
---
@@ -169,7 +170,17 @@ public void clear(W window, TriggerContext ctx) throws
Exception {}
* function (function is
not part os a KeyedStream).
*/
S getPartitionedState(StateDescriptor
stateDescriptor);
-
+
+ /**
+*
--- End diff --
extra line
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3758] Add possibility to register accum...
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/1979#discussion_r63047546
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/functions/util/AbstractRuntimeUDFContext.java
---
@@ -133,13 +132,24 @@ public DoubleCounter getDoubleCounter(String name) {
accumulators.put(name, accumulator);
}
- @SuppressWarnings("unchecked")
--- End diff --
This should stay to prevent warnings, should also be added to the other
`getAccumulator` method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3758] Add possibility to register accum...
Github user aljoscha commented on a diff in the pull request: https://github.com/apache/flink/pull/1979#discussion_r63047435 --- Diff: flink-core/src/main/java/org/apache/flink/api/common/functions/util/AbstractRuntimeUDFContext.java --- @@ -43,9 +38,13 @@ import org.apache.flink.api.common.state.ValueStateDescriptor; import org.apache.flink.api.common.typeinfo.TypeInformation; import org.apache.flink.core.fs.Path; - import static org.apache.flink.util.Preconditions.checkNotNull; +import java.io.Serializable; --- End diff -- unrelated changes. the order of imports should stay the same, even though it might be arbitrary ð --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-3758] Add possibility to register accum...
Github user knaufk closed the pull request at: https://github.com/apache/flink/pull/1979 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-3758] Add possibility to register accum...
GitHub user knaufk reopened a pull request:
https://github.com/apache/flink/pull/1979
[FLINK-3758] Add possibility to register accumulators in custom triggers
Thanks for contributing to Apache Flink. Before you open your pull request,
please take the following check list into consideration.
If your changes take all of the items into account, feel free to open your
pull request. For more information and/or questions please refer to the [How To
Contribute guide](http://flink.apache.org/how-to-contribute.html).
In addition to going through the list, please provide a meaningful
description of your changes.
- [ ] General
- The pull request references the related JIRA issue ("[FLINK-XXX] Jira
title text")
- The pull request addresses only one issue
- Each commit in the PR has a meaningful commit message (including the
JIRA id)
- [ ] Documentation
- Documentation has been added for new functionality
- Old documentation affected by the pull request has been updated
- JavaDoc for public methods has been added
- [ ] Tests & Build
- Functionality added by the pull request is covered by tests
- `mvn clean verify` has been executed successfully locally or a Travis
build has passed
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/knaufk/flink FLINK-3758
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/1979.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1979
commit d093fdd6bd22382cde3ae202adfd6d407caac0e8
Author: Konstantin Knauf
Date: 2016-05-03T20:45:48Z
Added getOrDefaultAccumulator to RuntimeContext
commit 8fc4d39bbb5c76a1b0cc0a4e93522e59fb2135bd
Author: Konstantin Knauf
Date: 2016-05-03T20:46:17Z
Exposed getOrDefaultAccumulator in WindowContext
commit 08df4fb00a500f8a131b4364f94c264c0b51e6b8
Author: Konstantin Knauf
Date: 2016-05-05T16:42:10Z
Added simple test for Accumulators in Triggers
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [Flink 3758] Add possibility to register accum...
GitHub user knaufk opened a pull request:
https://github.com/apache/flink/pull/1979
[Flink 3758] Add possibility to register accumulators in custom triggers
1 of 1
Thanks for contributing to Apache Flink. Before you open your pull request,
please take the following check list into consideration.
If your changes take all of the items into account, feel free to open your
pull request. For more information and/or questions please refer to the [How To
Contribute guide](http://flink.apache.org/how-to-contribute.html).
In addition to going through the list, please provide a meaningful
description of your changes.
- [ ] General
- The pull request references the related JIRA issue ("[FLINK-XXX] Jira
title text")
- The pull request addresses only one issue
- Each commit in the PR has a meaningful commit message (including the
JIRA id)
- [ ] Documentation
- Documentation has been added for new functionality
- Old documentation affected by the pull request has been updated
- JavaDoc for public methods has been added
- [ ] Tests & Build
- Functionality added by the pull request is covered by tests
- `mvn clean verify` has been executed successfully locally or a Travis
build has passed
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/knaufk/flink FLINK-3758
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/1979.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1979
commit d093fdd6bd22382cde3ae202adfd6d407caac0e8
Author: Konstantin Knauf
Date: 2016-05-03T20:45:48Z
Added getOrDefaultAccumulator to RuntimeContext
commit 8fc4d39bbb5c76a1b0cc0a4e93522e59fb2135bd
Author: Konstantin Knauf
Date: 2016-05-03T20:46:17Z
Exposed getOrDefaultAccumulator in WindowContext
commit 08df4fb00a500f8a131b4364f94c264c0b51e6b8
Author: Konstantin Knauf
Date: 2016-05-05T16:42:10Z
Added simple test for Accumulators in Triggers
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
