-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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 core version in UserAgent at all time #32242
Conversation
self._user_agent = "azsdk-python-{} Python/{} ({})".format( | ||
sdk_moniker, platform.python_version(), platform.platform() | ||
self._user_agent = "azsdk-python-{} {} Python/{} ({})".format( | ||
sdk_moniker, core_version, platform.python_version(), platform.platform() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two scenarios it may impact:
- If the telemetry data parser uses strict schema, it may cause breakings.
- If user appends their ua string (by calling
add_user_agent
method or using env var), it may cause data loss.
Do we need to be concerned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xiangyan99 for # 2 would the data loss be around telemetry ( or others ) who are looking for the old UA string ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the ua string is too long, it will be truncated.
Because we insert rather than appending here, the most end data will be truncated which could be appended by user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#1 is fair, maybe we put it at the end instead. @johanste any strong opinions here?
#2 I think is on the customer, there is no size for UserAgent in the HTTP spec, if people decided to truncate and read only a specific part, this is on them. Especially here we're adding roughly 12 bytes, which is not a lot.
API change check API changes are not detected in this pull request. |
Hi @lmazuel. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days. |
Hi @lmazuel. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing |
We want to always have the core version in the user-agent, for telemetry purposes.
This is a simple and stupid approach, meaning if calling SDK do not pass "sdk_monikers" you get the core version twice, but I think it's not important since: