You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "manisin (via GitHub)" <gi...@apache.org> on 2023/03/22 23:37:31 UTC
[GitHub] [iceberg] manisin opened a new pull request, #7176: Add unique JDBC application identifier and user agent heade
manisin opened a new pull request, #7176:
URL: https://github.com/apache/iceberg/pull/7176
Make JDBC application identifier unique for each catalog initialization and add another JDBC property to add the application identifier in the JDBC web request's user agent header.
Also updating to the latest version of snowflake's JDBC driver.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] danielcweeks commented on a diff in pull request #7176: Add unique JDBC application identifier and user agent header
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #7176:
URL: https://github.com/apache/iceberg/pull/7176#discussion_r1186442273
##########
snowflake/src/main/java/org/apache/iceberg/snowflake/SnowflakeCatalog.java:
##########
@@ -114,9 +121,13 @@ public void initialize(String name, Map<String, String> properties) {
cnfe);
}
+ String uniqueId = UUID.randomUUID().toString().replace("-", "").substring(0, UNIQUE_ID_LENGTH);
Review Comment:
Rather than a unique identifier, I would suggest using the build info: `IcebergBuild.fullVersion()` and `IcebergBuild.gitCommitShortId()` so you know what client version and build is being used.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on pull request #7176: Add unique JDBC application identifier and user agent header
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on PR #7176:
URL: https://github.com/apache/iceberg/pull/7176#issuecomment-1542582162
Moving the PR to https://github.com/apache/iceberg/pull/7580 due to bad rebase.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on pull request #7176: Add unique JDBC application identifier and user agent header
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on PR #7176:
URL: https://github.com/apache/iceberg/pull/7176#issuecomment-1489513710
@danielcweeks gentle ping to review this
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin closed pull request #7176: Add unique JDBC application identifier and user agent header
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin closed pull request #7176: Add unique JDBC application identifier and user agent header
URL: https://github.com/apache/iceberg/pull/7176
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on a diff in pull request #7176: Add unique JDBC application identifier and user agent header
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on code in PR #7176:
URL: https://github.com/apache/iceberg/pull/7176#discussion_r1190224787
##########
snowflake/src/main/java/org/apache/iceberg/snowflake/SnowflakeCatalog.java:
##########
@@ -114,9 +121,13 @@ public void initialize(String name, Map<String, String> properties) {
cnfe);
}
+ String uniqueId = UUID.randomUUID().toString().replace("-", "").substring(0, UNIQUE_ID_LENGTH);
Review Comment:
Thanks for the review @danielcweeks ! I have added iceberg version and commit. The Unique identifier part is still required to correlate header with JDBC sessions in our telemetry. Also I have to move this PR to https://github.com/apache/iceberg/pull/7580 due to bad rebase.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a diff in pull request #7176: Add unique JDBC application identifier and user agent header
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7176:
URL: https://github.com/apache/iceberg/pull/7176#discussion_r1145865995
##########
snowflake/src/main/java/org/apache/iceberg/snowflake/SnowflakeCatalog.java:
##########
@@ -114,9 +120,11 @@ public void initialize(String name, Map<String, String> properties) {
cnfe);
}
+ String uniqueAppIdentifier = APP_IDENTIFIER + "-" + randomAlphaNumeric(UNIQUE_ID_LENGTH);
Review Comment:
what about using a random UUID?
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a diff in pull request #7176: Add unique JDBC application identifier and user agent header
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7176:
URL: https://github.com/apache/iceberg/pull/7176#discussion_r1147179869
##########
snowflake/src/main/java/org/apache/iceberg/snowflake/SnowflakeCatalog.java:
##########
@@ -48,7 +49,12 @@ public class SnowflakeCatalog extends BaseMetastoreCatalog
// Specifies the name of a Snowflake's partner application to connect through JDBC.
// https://docs.snowflake.com/en/user-guide/jdbc-parameters.html#application
private static final String JDBC_APPLICATION_PROPERTY = "application";
+ // Add a suffix to user agent header for the web requests made by the jdbc driver.
+ private static final String JDBC_USER_AGENT_PROPERTY = "user_agent_suffix";
Review Comment:
nit: it's a bit non-intuitive that the property says "user agent" but the actual value says "user agent suffix"
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org