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

SNOW-830955 Use default SDK version in User-Agent #513

Merged
merged 1 commit into from
Jun 6, 2023

Conversation

sfc-gh-lsembera
Copy link
Contributor

While constructing HTTP User-Agent header, take the SDK version from code instead from a file on classpath. It is less prone to accidental classpath overrides. The consequence of this is that we must now keep the DEFAULT_VERSION in sync with the project version defined in Maven, which we are doing already anyway (a new test has been added to verify this).

@codecov
Copy link

codecov bot commented Jun 2, 2023

Codecov Report

Merging #513 (a10d638) into master (8f78df6) will increase coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #513      +/-   ##
==========================================
+ Coverage   74.88%   74.97%   +0.08%     
==========================================
  Files          77       77              
  Lines        4771     4756      -15     
  Branches      419      418       -1     
==========================================
- Hits         3573     3566       -7     
+ Misses       1017     1010       -7     
+ Partials      181      180       -1     
Impacted Files Coverage Δ
...et/snowflake/ingest/connection/RequestBuilder.java 76.30% <100.00%> (+0.76%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sfc-gh-lsembera sfc-gh-lsembera marked this pull request as ready for review June 2, 2023 11:44
@sfc-gh-xhuang
Copy link
Contributor

sfc-gh-xhuang commented Jun 3, 2023

Thanks!

Can you quickly validate the version number shows up as expected in snowhouse for the account you tested this change on:

Copy link
Contributor

@sfc-gh-tzhang sfc-gh-tzhang left a comment

Choose a reason for hiding this comment

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

lgtm!

While constructing HTTP User-Agent header, take the SDK version from code instead from a file on classpath. It is less prone to accidental classpath overrides. The consequence of this is that we must now keep the DEFAULT_VERSION in sync with the project version defined in Maven, which we are doing already anyway (a new test has been added to verify this).
@sfc-gh-lsembera
Copy link
Contributor Author

@sfc-gh-xhuang I verified the version is reported correctly.

@sfc-gh-lsembera sfc-gh-lsembera merged commit 083fce0 into master Jun 6, 2023
11 of 12 checks passed
@sfc-gh-lsembera sfc-gh-lsembera deleted the lsembera/wrong-version branch June 6, 2023 10:56
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.

None yet

5 participants