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-35358][clients] Reintroduce recursive JAR listing in classpath load from "usrlib" #24791

Merged
merged 1 commit into from
May 30, 2024

Conversation

ferenc-csaky
Copy link
Contributor

What is the purpose of the change

With e63aa12 in Flink 1.19 an unintentional change were introduced, that now the DefaultPackagedProgramRetriever only loads JARs from the root level of the usrlib directory.

This change reverts that behavior to search directories in a recursive manner, and adds a test case to cover such scenario.

Brief change log

  • Flies#list -> Files#walk in DefaultPackagedProgramRetriever.
  • Add test case in DefaultPackagedProgramRetrieverITCase.

Verifying this change

Added relevant IT case.

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, Kubernetes/Yarn, ZooKeeper: yes
  • The S3 file system connector: no

@flinkbot
Copy link
Collaborator

flinkbot commented May 15, 2024

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

Copy link
Contributor

@mbalassi mbalassi left a comment

Choose a reason for hiding this comment

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

LGTM, will merge tomorrow if no objections until then.

@rkthtrifork
Copy link

@mbalassi @ferenc-csaky can we merge this and #24792 ?

@ferenc-csaky
Copy link
Contributor Author

@mbalassi @ferenc-csaky can we merge this and #24792 ?

Yes, they should be merged.

@hlteoh37 hlteoh37 merged commit 853989b into apache:master May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants