-
Notifications
You must be signed in to change notification settings - Fork 650
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
Update user agent for NetworkClient() #11001
Update user agent for NetworkClient() #11001
Conversation
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.
Thanks for tackling this! Could you make one small change, for simplicity?
Build Artifacts
|
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.
Thanks for resolving my last feedback. It looks good.
Now I see the test is failing, so just a couple fixes for the test to pass and this to be merged
@bjester Thanks again for checking. I addressed the test & lint issues. btw I was thinking I could either:
lmk what's the recommended approach. |
@mhasbini There are still some linting failures that I can see. If you install Could you please comment on this issue so I can make you the assignee? Yes you're right, there are several usages of Refactoring existing usage of |
@bjester Thanks for the clarification! |
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.
Thank you @mhasbini!
Summary
Update user agent for
NetworkClient()
to include the project name with its current version.I need guidance in deciding how to implement this change for files that includes
requests
directly. I have two approaches:NetworkClient()
requests
that have this custom user agent.lmk what's the recommended approach.
References
Fixes #10995
Reviewer guidance
Testing checklist
PR process
Reviewer checklist
yarn
andpip
)