You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "FangYongs (via GitHub)" <gi...@apache.org> on 2023/05/06 01:54:13 UTC
[GitHub] [flink] FangYongs opened a new pull request, #22533: [FLINK-31687][jdbc-driver] Get rid of flink-core for jdbc driver
FangYongs opened a new pull request, #22533:
URL: https://github.com/apache/flink/pull/22533
## What is the purpose of the change
This PR aims to get rid of flink-core for flink jdbc driver
## Brief change log
- Get Map<String, String> instead of Configuratioin in Executor.getSessionConfig
- Get rid of flink-core in jdbc driver
## Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (yes / no) no
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / no) no
- The serializers: (yes / no / don't know) no
- The runtime per-record code paths (performance sensitive): (yes / no / don't know) no
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes / no / don't know) no
- The S3 file system connector: (yes / no / don't know) no
## Documentation
- Does this pull request introduce a new feature? (yes / no) no
- If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] libenchao commented on a diff in pull request #22533: [FLINK-31687][jdbc-driver] Get rid of flink-core for jdbc driver
Posted by "libenchao (via GitHub)" <gi...@apache.org>.
libenchao commented on code in PR #22533:
URL: https://github.com/apache/flink/pull/22533#discussion_r1189370293
##########
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/Executor.java:
##########
@@ -46,7 +46,7 @@ static Executor create(
*
* @return the session configuration.
*/
- ReadableConfig getSessionConfig();
+ Map<String, String> getSessionConfig();
Review Comment:
Is this a public api? Although this is not clearly annotated as 'api', we may need to be careful about this change. CC @fsk119 what do you think about this since you are the original author of this class.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] FangYongs commented on pull request #22533: [FLINK-31687][jdbc-driver] Get rid of flink-core for jdbc driver
Posted by "FangYongs (via GitHub)" <gi...@apache.org>.
FangYongs commented on PR #22533:
URL: https://github.com/apache/flink/pull/22533#issuecomment-1541426833
Hi @fsk119 , please help to review this PR when you are free, thanks
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #22533: [FLINK-31687][jdbc-driver] Get rid of flink-core for jdbc driver
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #22533:
URL: https://github.com/apache/flink/pull/22533#issuecomment-1536981075
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d0b114541c6f5c7c477f37712999fc1a24f8cbc4",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "d0b114541c6f5c7c477f37712999fc1a24f8cbc4",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d0b114541c6f5c7c477f37712999fc1a24f8cbc4 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] fsk119 commented on a diff in pull request #22533: [FLINK-31687][jdbc-driver] Get rid of flink-core for jdbc driver
Posted by "fsk119 (via GitHub)" <gi...@apache.org>.
fsk119 commented on code in PR #22533:
URL: https://github.com/apache/flink/pull/22533#discussion_r1193272280
##########
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/Executor.java:
##########
@@ -46,7 +46,7 @@ static Executor create(
*
* @return the session configuration.
*/
- ReadableConfig getSessionConfig();
+ Map<String, String> getSessionConfig();
Review Comment:
Yes. I think external system may rely on this. Can we add another method, e.g. getSessionConfigMap?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] FangYongs commented on pull request #22533: [FLINK-31687][jdbc-driver] Get rid of flink-core for jdbc driver
Posted by "FangYongs (via GitHub)" <gi...@apache.org>.
FangYongs commented on PR #22533:
URL: https://github.com/apache/flink/pull/22533#issuecomment-1537803429
Hi @libenchao Please help to review this PR when you're free, thanks
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] FangYongs commented on a diff in pull request #22533: [FLINK-31687][jdbc-driver] Get rid of flink-core for jdbc driver
Posted by "FangYongs (via GitHub)" <gi...@apache.org>.
FangYongs commented on code in PR #22533:
URL: https://github.com/apache/flink/pull/22533#discussion_r1197509134
##########
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/Executor.java:
##########
@@ -46,7 +46,7 @@ static Executor create(
*
* @return the session configuration.
*/
- ReadableConfig getSessionConfig();
+ Map<String, String> getSessionConfig();
Review Comment:
DONE
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] libenchao closed pull request #22533: [FLINK-31687][jdbc-driver] Get rid of flink-core for jdbc driver
Posted by "libenchao (via GitHub)" <gi...@apache.org>.
libenchao closed pull request #22533: [FLINK-31687][jdbc-driver] Get rid of flink-core for jdbc driver
URL: https://github.com/apache/flink/pull/22533
--
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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org