-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[FLINK-7711] Port JarListHandler to WebMonitorEndpoint #5209
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution to Flink @zjureel! I left a comment about the duplicated code from PackagedProgram. Also I think that we should implement and merge FLINK-7713 first so that we are able verify the functionality end-to-end. Let me know what you think.
| /** | ||
| * Jar file validation tool class for {@link Program}. | ||
| */ | ||
| public class JarWithProgramUtils { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see that you had to copy a lot of code from PackagedProgram because flink-runtime does not have a dependency on flink-clients. Also, JobWithJars had to be moved to the flink-runtime module. I think we should try to avoid duplicating the code in PackagedProgram.
After talking to @tillrohrmann offline, we think that we could achieve this by putting the new JarUploadHandler into the flink-runtime-web module. This module already has a dependency to flink-clients and flink-runtime. In WebMonitorEndpoint, the jar handlers could be dynamically loaded according to whether flink-runtime-web is in the classpath or not; similar to the StaticFileServerHandler (see WebMonitorUtils.tryLoadWebContent()).
What do you think?
This closes apache#5209. This closes apache#5455.
This closes apache#5209. This closes apache#5455.
This closes apache#5209. This closes apache#5455.
This closes apache#5209. This closes apache#5455.
This closes apache#5209. This closes apache#5455.
This closes apache#5209. This closes apache#5455.
This closes apache#5209. This closes apache#5455.
What is the purpose of the change
Port JarListHandler to WebMonitorEndpoint
Brief change log
Verifying this change
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (no)Documentation