Skip to content

Conversation

@tisonkun
Copy link
Member

What is the purpose of the change

(Copy from JIRA)
Description
JarFileCreatorTest fails when run with Java 9. Because JarFileCreator is not used in production, the class and its tests can be deleted.

Acceptance Criteria

  • JarFileCreator and JarFileCreatorTest are deleted
  • Usage of JarFileCreator in ClassLoaderUtilsTest#testWithURLClassLoader is replaced with an alternative

Brief change log

  • Delete JarFileCreator and JarFileCreatorTest
  • Replace usage of JarFileCreator in ClassLoaderUtilsTest#testWithURLClassLoader with a specific util method.

Verifying this change

Basically it is a trivial code cleanup and we ensure ClassLoaderUtilsTest#testWithURLClassLoader still valid.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
  • The serializers: (no)
  • The runtime per-record code paths (performance sensitive): (no)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
  • The S3 file system connector: (no)

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

cc @GJL

@GJL
Copy link
Member

GJL commented Jan 14, 2019

Thanks @tisonkun , I will have a look.

@GJL GJL self-assigned this Jan 14, 2019
@tisonkun
Copy link
Member Author

Thanks @GJL for being an assignee.

@aljoscha aljoscha requested a review from GJL January 17, 2019 09:25
@GJL GJL requested a review from aljoscha January 17, 2019 11:02
Copy link
Contributor

@aljoscha aljoscha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me this looks good! @GJL can you confirm whether this will work with Java 9 (and future versions)?

@GJL
Copy link
Member

GJL commented Jan 22, 2019

@aljoscha ClassLoaderUtilsTest#testWithURLClassLoader passes with Java 9 now. If you think this PR is fine, please merge.

GJL added a commit to GJL/flink that referenced this pull request Jan 25, 2019
@GJL
Copy link
Member

GJL commented Jan 25, 2019

Merging myself...

GJL added a commit to GJL/flink that referenced this pull request Jan 25, 2019
GJL added a commit to GJL/flink that referenced this pull request Jan 26, 2019
@asfgit asfgit closed this in 3fdb2af Jan 26, 2019
@tisonkun tisonkun deleted the FLINK-11316 branch January 26, 2019 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants