Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add user-agent parameter to config #5

Merged
merged 9 commits into from
Aug 16, 2024

Conversation

yu-kioo
Copy link
Contributor

@yu-kioo yu-kioo commented Aug 7, 2024

overview

add optional user-agent parameter

@yu-kioo yu-kioo changed the title add user-agent parameter Add user-agent parameter to config Aug 7, 2024
@@ -75,6 +90,13 @@ protected JdbcInputConnection newConnection(PluginTask task) throws SQLException
props.put("ConnSchema", t.getSchemaName().get());
}
props.putAll(t.getOptions());
// overwrite UserAgentEntry property if the same property is set in options
if (t.getUserAgentEntry() != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

t.getUserAgentEntry always seems to be not null.
You might want to delete this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current code did not guarantee the behavior of 2 below, so changed condition.

  • 1: options is not set & user-agent is not set => unknown/0.0.0
  • 2: options is set & user-agent is not set => value of options
  • 3: options is not set & user-agent is set => value of user-agent
  • 4: options is set, user-agent is set => value of user-agent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have decided to always overwrite the UserAgentEntry value in options by user-agent, so please ignore the above comment. I removed condition as you commented.

Comment on lines 95 to 98
String product_name = t.getUserAgentEntry().getProductName();
String product_version = t.getUserAgentEntry().getProductVersion();

props.put("UserAgentEntry", product_name + "/" + product_version);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Java's variable names are usually camel case.

product_name -> productName
product_version -> productVersion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, fixed.

@yu-kioo yu-kioo requested a review from chikamura August 14, 2024 04:40
@yu-kioo yu-kioo self-assigned this Aug 15, 2024
Copy link
Contributor

@chikamura chikamura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yu-kioo yu-kioo merged commit 91d572e into main Aug 16, 2024
2 checks passed
@yu-kioo yu-kioo deleted the add-user-agent-parameter-to-config branch August 16, 2024 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants