[FLINK-31223][sqlgateway] Introduce getFlinkConfigurationOptions to #24741
Conversation
|
@reswqa As discussed please could you merge |
|
@flinkbot run azure |
|
@reswqa junit errors in the pr build - I will investigate - update it is clean now. |
|
@reswqa are you ok to merge this please? |
|
Sorry for the delay. The ci build seems also have some problem, would you mind taking a look? Thanks. |
@reswqa no worries - it looks clean now - LGTM . This is something we are looking to get into our build as a priority, it would be fabulous if you could merge this asap. Thank you very much for your support. |
|
I think we should also including 4e6dbe2. |
|
@reswqa If you have strong views I can include it in this one. |
|
We should include it as flink has nightly ci that check jdk17 build also. Otherwise this pr will broke ci pipeline. |
@reswqa thanks for the clarification :-) |
|
@reswqa I have backported the test code as requested. |
|
@reswqa I have done the same for the 1.19 backport |
|
@flinkbot run azure |
|
@reswqa I am seeing errors in the CI junits - like it is not using the -e option in some other tests. Investigating. |
Signed-off-by: David Radley <david_radley@uk.ibm.com>
|
@reswqa the CI output shows that the config.yaml is not picked up. This was moved into the base test calls by the fix. On the face of it it looks like the @beforeeach is not being driven for each test, so the yaml file is not present resulting in the error. Continuing to investigate. |
|
|
||
| // prepare conf dir | ||
| File confFolder = Files.createTempDirectory(tempFolder, "conf").toFile(); | ||
| File confYaml = new File(confFolder, "config.yaml"); |
There was a problem hiding this comment.
IIRC, config.yaml will not be recognized as a valid flink configuration file in 1.18. It should be flink-conf.yaml.
Back port of #22026 to Flink 1.18.
What is the purpose of the change
SqlClient now uses SqlGateway for job submission. It will connect to the gateway's RestServer through the RestClient to interact. when we create the RestClient, we load all the configuration options, but when we create the rest server, we only load the EndpointOptions. As a result, if we want to ensure the security of the connection through SSL, the client will enable SSL, but the server does not. The problem of parameter inconsistency will lead to potential problems in the future. We'd better avoid it directly.
Brief change log
Pass all flink parameters to the SqlGatewayRestEndpoint.
Verifying this change
This change added tests and can be verified by SqlClientSSLTest.
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): yes
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: no
The S3 file system connector: no
Documentation
Does this pull request introduce a new feature? no