kl0u opened a new pull request #9974: [FLINK-14501][FLINK-14502] Decouple ClusterDescriptor/ClusterSpecification from CommandLine URL: https://github.com/apache/flink/pull/9974 ## What is the purpose of the change This change decouples the cluster/cluster client creation from the command line, by putting a configuration object in-between. Previously, the `CustomCommandLine` was responsible for the cluster client creation based on the methods: ``` ClusterDescriptor<T> createClusterDescriptor(CommandLine commandLine) throws FlinkException; T getClusterId(CommandLine commandLine); ClusterSpecification getClusterSpecification(CommandLine commandLine) throws FlinkException; ``` This was an unnecessary coupling and also bad from a separation of concerns standpoint. Ideally, the `CLI` should be responsible for parsing the command line options only. This PR aims at doing exactly that. *Previously the command line options were directly passed to the descriptor and the specification from the `CustomCommandLine`. *Now the `CustomCommandLine` only has a method: ``` Configuration applyCommandLineOptionsToConfiguration(CommandLine commandLine) throws FlinkException; ``` which maps the command-line options to a configuration object and this object is then used to: 1. instantiate the correct `ClusterClientFactory` (Yarn or Standalone currently) based on the `execution.target` config option. 2. create the adequate `ClusterDescriptor`. ## Brief change log The previous `CustomCommandLine` is now only responsible for translating the CLI-options to a configuration. A new `ClusterClientFactory` abstraction is introduced with the following methods that take care of cluster creation/ retrieval: ``` /** * Returns {@code true} if the current {@link ClusterClientFactory} is compatible with the provided configuration, {@code false} otherwise. */ boolean isCompatibleWith(Configuration configuration); ClusterDescriptor<ClusterID> createClusterDescriptor(Configuration configuration); @Nullable ClusterID getClusterId(Configuration configuration); ClusterSpecification getClusterSpecification(Configuration configuration); ``` ## Verifying this change This change is mainly covered by existing test. Some additional tests are added for the service discovery part. ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (yes / **no**) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (**yes** / no) - The serializers: (yes / **no** / don't know) - The runtime per-record code paths (performance sensitive): (yes / **no** / don't know) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (**yes** / no / don't know) - The S3 file system connector: (yes / **no** / don't know) ## Documentation - Does this pull request introduce a new feature? (**yes** / no) - If yes, how is the feature documented? (not applicable / **docs** / **JavaDocs** / not documented) **This PR is part of FLIP-73 and the new configuration options it introduces are under voting process with FLIP-81. This PR is here for pre-reviewing but it will only be merged after FLIP-81 is merged.**
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to 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
