zentol commented on a change in pull request #7717: [FLINK-11533] [container]
Add option parse JAR manifest for jobClassName
URL: https://github.com/apache/flink/pull/7717#discussion_r258843640
##########
File path:
flink-container/src/main/java/org/apache/flink/container/entrypoint/StandaloneJobClusterConfigurationParserFactory.java
##########
@@ -49,7 +49,9 @@
.required(true)
Review comment:
When I wrote my suggestion I forgot that we don't pass a jar by default like
for normal submissions 🤦♂️
The `from-jar:` workaround seems rather unusual to me. It also makes the
`jobClassName` field in the `StandaloneJobClusterConfiguration` ambiguous; it
may be a class name or the path to a file with a custom scheme, which are
entirely different things.
I would be in favor of a separate argument; the added overhead for ensuring
that one of them is set is rather low and I don't want to start adding
workarounds just because the underlying library doesn't support case X
out-of-the-box. This option could be named `--manifest-jar` or `--from-jar`
without a shorthand (to retain the `from-jar` principle being visible).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services