Re: [PR] [FLINK-35176][Connector/JDBC] Support property authentication connection for JDBC catalog & dynamic table [flink-connector-jdbc]

2024-05-06 Thread via GitHub 


1996fanrui merged PR #116:
URL: https://github.com/apache/flink-connector-jdbc/pull/116


-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-35176][Connector/JDBC] Support property authentication connection for JDBC catalog & dynamic table [flink-connector-jdbc]

2024-05-02 Thread via GitHub 


RocMarshal commented on PR #116:
URL: 
https://github.com/apache/flink-connector-jdbc/pull/116#issuecomment-2092246949

   Thanks @eskabetxe for the comments. Updated.


-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-35176][Connector/JDBC] Support property authentication connection for JDBC catalog & dynamic table [flink-connector-jdbc]

2024-04-30 Thread via GitHub 


eskabetxe commented on code in PR #116:
URL: 
https://github.com/apache/flink-connector-jdbc/pull/116#discussion_r1584601921


##
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/catalog/AbstractJdbcCatalog.java:
##
@@ -88,32 +92,49 @@ public abstract class AbstractJdbcCatalog extends 
AbstractCatalog {
 private static final Logger LOG = 
LoggerFactory.getLogger(AbstractJdbcCatalog.class);
 
 protected final ClassLoader userClassLoader;
-protected final String username;
-protected final String pwd;
 protected final String baseUrl;
 protected final String defaultUrl;
+protected final Properties connectionProperties;
 
+@Deprecated
 public AbstractJdbcCatalog(
 ClassLoader userClassLoader,
 String catalogName,
 String defaultDatabase,
 String username,
 String pwd,
 String baseUrl) {
+this(
+userClassLoader,
+catalogName,
+defaultDatabase,
+baseUrl,
+getBriefAuthProperties(username, pwd));
+}
+
+public AbstractJdbcCatalog(
+ClassLoader userClassLoader,
+String catalogName,
+String defaultDatabase,
+String baseUrl,
+Properties connectionProperties) {
 super(catalogName, defaultDatabase);
 
 checkNotNull(userClassLoader);
-checkArgument(!StringUtils.isNullOrWhitespaceOnly(username));
-checkArgument(!StringUtils.isNullOrWhitespaceOnly(pwd));
 checkArgument(!StringUtils.isNullOrWhitespaceOnly(baseUrl));
 
 JdbcCatalogUtils.validateJdbcUrl(baseUrl);
 
 this.userClassLoader = userClassLoader;
-this.username = username;
-this.pwd = pwd;
 this.baseUrl = baseUrl.endsWith("/") ? baseUrl : baseUrl + "/";
 this.defaultUrl = this.baseUrl + defaultDatabase;
+this.connectionProperties = 
Preconditions.checkNotNull(connectionProperties);
+if 
(StringUtils.isNullOrWhitespaceOnly(connectionProperties.getProperty(USER_KEY)))
 {

Review Comment:
   we should maintain the current checks
   ```
   
checkArgument(!StringUtils.isNullOrWhitespaceOnly(connectionProperties.getProperty(USER_KEY)));
   
checkArgument(!StringUtils.isNullOrWhitespaceOnly(connectionProperties.getProperty(PASSWORD_KEY)));
   ```



-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-35176][Connector/JDBC] Support property authentication connection for JDBC catalog & dynamic table [flink-connector-jdbc]

2024-04-27 Thread via GitHub 


RocMarshal commented on PR #116:
URL: 
https://github.com/apache/flink-connector-jdbc/pull/116#issuecomment-2081320858

   Hi, @eskabetxe Would you mind having take a look if you had the free time ? 
Many thx!


-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-35176][Connector/JDBC] Support property authentication connection for JDBC catalog & dynamic table [flink-connector-jdbc]

2024-04-27 Thread via GitHub 


RocMarshal commented on PR #116:
URL: 
https://github.com/apache/flink-connector-jdbc/pull/116#issuecomment-2080456998

   Thanks @caicancai for the review.
   Could @1996fanrui you help take a look if you had the free time ? 
   Thank you~


-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-35176][Connector/JDBC] Support property authentication connection for JDBC catalog & dynamic table [flink-connector-jdbc]

2024-04-27 Thread via GitHub 


caicancai commented on code in PR #116:
URL: 
https://github.com/apache/flink-connector-jdbc/pull/116#discussion_r1581771380


##
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/catalog/JdbcCatalog.java:
##
@@ -77,17 +81,42 @@ public JdbcCatalog(
 String pwd,
 String baseUrl,
 String compatibleMode) {
-super(userClassLoader, catalogName, defaultDatabase, username, pwd, 
baseUrl);
+this(
+userClassLoader,
+catalogName,
+defaultDatabase,
+baseUrl,
+compatibleMode,
+getBriefAuthProperties(username, pwd));
+}
+
+/**
+ * Creates a JdbcCatalog.

Review Comment:
   I mean why the comment is Creates a JdbcCatalog, not Create a JdbcCatalog



-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-35176][Connector/JDBC] Support property authentication connection for JDBC catalog & dynamic table [flink-connector-jdbc]

2024-04-26 Thread via GitHub 


RocMarshal commented on code in PR #116:
URL: 
https://github.com/apache/flink-connector-jdbc/pull/116#discussion_r1580440708


##
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/catalog/JdbcCatalog.java:
##
@@ -29,18 +29,22 @@
 import org.apache.flink.table.catalog.exceptions.TableNotExistException;
 
 import java.util.List;
+import java.util.Properties;
+
+import static 
org.apache.flink.connector.jdbc.JdbcConnectionOptions.getBriefAuthProperties;
 
 /** Catalogs for relational databases via JDBC. */
 @PublicEvolving
 public class JdbcCatalog extends AbstractJdbcCatalog {
 
 private final AbstractJdbcCatalog internal;
 
+@Deprecated
 /**
  * Creates a JdbcCatalog.

Review Comment:
   Anchor: A



##
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/catalog/JdbcCatalog.java:
##
@@ -77,17 +81,42 @@ public JdbcCatalog(
 String pwd,
 String baseUrl,
 String compatibleMode) {
-super(userClassLoader, catalogName, defaultDatabase, username, pwd, 
baseUrl);
+this(
+userClassLoader,
+catalogName,
+defaultDatabase,
+baseUrl,
+compatibleMode,
+getBriefAuthProperties(username, pwd));
+}
+
+/**
+ * Creates a JdbcCatalog.

Review Comment:
   @caicancai Thanks for the comment.
   I typed it in based on the style like 'Anchor: A'.
   Maybe I get the wrong meaning from the comment. Would you mind clarifying 
more details ?  many thx. :)



-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-35176][Connector/JDBC] Support property authentication connection for JDBC catalog & dynamic table [flink-connector-jdbc]

2024-04-25 Thread via GitHub 


caicancai commented on code in PR #116:
URL: 
https://github.com/apache/flink-connector-jdbc/pull/116#discussion_r1580352031


##
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/catalog/JdbcCatalog.java:
##
@@ -77,17 +81,42 @@ public JdbcCatalog(
 String pwd,
 String baseUrl,
 String compatibleMode) {
-super(userClassLoader, catalogName, defaultDatabase, username, pwd, 
baseUrl);
+this(
+userClassLoader,
+catalogName,
+defaultDatabase,
+baseUrl,
+compatibleMode,
+getBriefAuthProperties(username, pwd));
+}
+
+/**
+ * Creates a JdbcCatalog.

Review Comment:
   creates？



-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-35176][Connector/JDBC] Support property authentication connection for JDBC catalog & dynamic table [flink-connector-jdbc]

2024-04-25 Thread via GitHub 


RocMarshal closed pull request #116: [FLINK-35176][Connector/JDBC] Support 
property authentication connection for JDBC catalog & dynamic table
URL: https://github.com/apache/flink-connector-jdbc/pull/116


-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-35176][Connector/JDBC] Support property authentication connection for JDBC catalog & dynamic table [flink-connector-jdbc]

2024-04-25 Thread via GitHub 


RocMarshal commented on PR #116:
URL: 
https://github.com/apache/flink-connector-jdbc/pull/116#issuecomment-2077035421

   HI, @caicancai @GOODBOY008 Could you help have a review if you had the free 
time ? thx a lot.


-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-35176][Connector/JDBC] Support property authentication connection for JDBC catalog & dynamic table [flink-connector-jdbc]

2024-04-24 Thread via GitHub 


RocMarshal commented on PR #116:
URL: 
https://github.com/apache/flink-connector-jdbc/pull/116#issuecomment-2074135052

   blocked by https://github.com/apache/flink-connector-jdbc/pull/115


-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org