Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-3093] Introduce annotations for interface stability in remaining modules #1428

Closed

Conversation

rmetzger
Copy link
Contributor

@rmetzger rmetzger commented Dec 1, 2015

Please see #1426

This pull request depends on #1427

@aljoscha
Copy link
Contributor

aljoscha commented Dec 7, 2015

Should we also change names of stuff or open separate Jiras, PRs. I'm asking, for example, because of CheckpointNotifier which should be called CheckpointListener or something like it.

@aljoscha
Copy link
Contributor

aljoscha commented Dec 7, 2015

I created a patch of changes in the Stream API that I would propose:

stream-api-annotations.patch.txt

(you have to remove the .txt extension, github would not let me upload without it...)

@rmetzger
Copy link
Contributor Author

Thank you for your patch. I've added it to the PR.

Regarding your question: I would keep name changes separate from this PR. Once this one is merged, we can start changing names.

Once #1427 is merged, I'll rebase this PR and we can finalize it.

@fhueske
Copy link
Contributor

fhueske commented Dec 18, 2015

Created a PR for flink-java.

Summary

Changed to Experimental:

  • DataSink.sortLocalOutput (should be deprecated, IMO. See FLINK-3186)
  • DataSource.getSplitDataProperties
  • SplitDataProperties
  • DataSetUtils (not sure if we won't move methods out to core API, discussion please)

Changed to Public:

  • CsvReader
  • All subclasses of DataSet, incl. Operator
  • Grouping and subclasses
  • Either and EitherTypeInfo
  • ObjectArrayTypeInfo
  • FunctionAnnotation
  • DiscardingOF
  • TextIF and TextOF

@fhueske
Copy link
Contributor

fhueske commented Dec 18, 2015

Do we want to make the IOFormats Public as well?
Most of them are hidden by helper methods of the ExecutionEnvironment. However, they can also be used as regular input formats via ExecutionEnvironment.createInput().

@rmetzger rmetzger force-pushed the interface_stability_no_maven-rest branch from 969941b to 7668900 Compare January 7, 2016 17:53
@rmetzger
Copy link
Contributor Author

rmetzger commented Jan 7, 2016

I merged both pull requests by @fhueske and the patch by @aljoscha and rebased the code on the lastest master (with flink-core annotations merged).

@fhueske
Copy link
Contributor

fhueske commented Jan 10, 2016

Thanks for the update!
I'd like to do another pass over the DataSet Java and Scala classes next week.

@rmetzger
Copy link
Contributor Author

Cool, thank you.
I'll rebase the PR, and go over the APIs again as well

@rmetzger rmetzger force-pushed the interface_stability_no_maven-rest branch 2 times, most recently from bf140d6 to bb4f49e Compare January 21, 2016 10:02
@rmetzger rmetzger force-pushed the interface_stability_no_maven-rest branch from afaf220 to b27f351 Compare February 4, 2016 16:18
@rmetzger
Copy link
Contributor Author

rmetzger commented Feb 4, 2016

I went through all the classes in the relevant modules outside flink-core with @fhueske and annotated them properly.
I've rebased the changes and checked with the additions in between.

I'm going to merge this PR in the next ~12 hours if nobody objects.

@rmetzger rmetzger force-pushed the interface_stability_no_maven-rest branch from b27f351 to f341f73 Compare February 4, 2016 16:40
rmetzger added a commit to rmetzger/flink that referenced this pull request Feb 5, 2016
@asfgit asfgit closed this in b54499b Feb 5, 2016
subhankarb pushed a commit to subhankarb/flink that referenced this pull request Mar 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants