You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by rmetzger <gi...@git.apache.org> on 2016/05/27 10:31:48 UTC
[GitHub] flink pull request: [FLINK-3129] Add tooling to ensure interface s...
GitHub user rmetzger opened a pull request:
https://github.com/apache/flink/pull/2042
[FLINK-3129] Add tooling to ensure interface stability
This pull request adds a maven module for ensuring interface stability (japicmp).
I needed to revert some of the changes since 1.0:
- I added the `public static final String CONFIG_KEY` in the `ExecutionConfig` again.
- I made the `getMetricGroup()` in the `RuntimeContext` an evolving API.
- I added two deprecated config constants again.
- I reintroduced the deprecated `Key` interface.
Once this has been merged, I'll file JIRAs for 2.0 to remove these changes.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rmetzger/flink flink3129-third
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/2042.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 #2042
----
commit fd0e81558953f04b07dd5edaec58e9fe73903a7f
Author: Robert Metzger <rm...@apache.org>
Date: 2015-12-07T14:45:57Z
[FLINK-3129] Bring infrastructure for ensuring interface stability in place
commit b15d5b84dd714fb6660b51ce72f03f1f25c134b8
Author: Robert Metzger <rm...@apache.org>
Date: 2016-05-27T08:53:35Z
[FLINK-3129] Fix breaking changes in flink-core
commit b53b4805a70f2c0ae678bf41148c82e04b531417
Author: Robert Metzger <rm...@apache.org>
Date: 2016-05-27T10:09:45Z
[FLINK-3129] Fix ES2 issue
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3129] Add tooling to ensure interface s...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/2042
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3129] Add tooling to ensure interface s...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/2042#issuecomment-222139308
Thank you for the review.
Its very low overhead since the 1.0.0 jar's are cached in the local .m2 directory and the tool is doing the check really fast.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3129] Add tooling to ensure interface s...
Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/2042#issuecomment-222132940
Thanks for the pull request! Could you elaborate on how and where this will be integrated with our CI or release process?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3129] Add tooling to ensure interface s...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/2042#issuecomment-222248940
Okay, let me explain:
I decided to remove the field, even though its public, and strictly speaking a breaking API change. However, it was a mistake to mark the field as public (back then, when it was added). It was only used internally as a configuration key for storing the serialized execution config (so it was something only used by flink).
It doesn't have any use for our users at all, so I think its absolutely safe to remove it. Its also not mentioned in the documentation.
I've added this exclude to the API check to ignore the breaking change:
https://github.com/apache/flink/commit/b0acd97935cd21843bac3b9b5afa3662b52bb95d#diff-40616c4678c3fbfe07c0701505ce0567
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3129] Add tooling to ensure interface s...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/2042#issuecomment-222185242
@zentol I think your concern was addressed here: https://github.com/apache/flink/pull/2042/commits/c20efc6cda6ff705c72924213c90f618f7f40e51
Thanks all for the review!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3129] Add tooling to ensure interface s...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/2042#discussion_r64902741
--- Diff: flink-core/src/main/java/org/apache/flink/api/common/ExecutionConfig.java ---
@@ -82,6 +82,10 @@
private static final long DEFAULT_RESTART_DELAY = 10000L;
+ // This field was used as a key for storing the EC in the Job Configuration
+ @Deprecated
+ public static final String CONFIG_KEY = "runtime.config";
--- End diff --
this change belongs in the "Fix breaking changes in flink-core" commit, unless they will be squashed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3129] Add tooling to ensure interface s...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/2042#issuecomment-222134657
The plugin runs with every mvn verify call.
For each maven module, it will download the last stable release (in this case 1.0.0) and check it against the build (currently the 1.1-SNAPSHOT).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3129] Add tooling to ensure interface s...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the pull request:
https://github.com/apache/flink/pull/2042#issuecomment-222208597
no my concern wasn't even remotely addressed.
The public static String CONFIG_KEY was removed in commit 5a7f4e3395bfb06da496584be88501c328f6ac1d, violating the interface stability.
In commit 6c079360c60f47a336eda2f8936a6bddfb948058 you add CONFIG_KEY in the ExecutionConfig, while the subsequent commit b0acd97935cd21843bac3b9b5afa3662b52bb95d removes it again.
so now the public static key is still removed which shouldn't be the case, no?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3129] Add tooling to ensure interface s...
Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/2042#issuecomment-222138890
+1 Looks good to me.
Only wondering, how much will it affect our build time? We might want to run this in a dedicated profile.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---