-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[py] Set user_agent and extra_headers via ClientConfig #14718
Conversation
Signed-off-by: Viet Nguyen Duc <[email protected]>
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #14718 +/- ##
==========================================
+ Coverage 58.48% 59.22% +0.74%
==========================================
Files 86 91 +5
Lines 5270 5874 +604
Branches 220 266 +46
==========================================
+ Hits 3082 3479 +397
- Misses 1968 2129 +161
- Partials 220 266 +46 ☔ View full report in Codecov by Sentry. |
CI Failure Feedback 🧐(Checks updated until commit 633b436)
|
Signed-off-by: Viet Nguyen Duc <[email protected]>
b40dd0e
to
02cc950
Compare
Signed-off-by: Viet Nguyen Duc <[email protected]>
da57c51
to
5b63c44
Compare
using basic with username and password.""" | ||
using basic with username and password. | ||
|
||
Support values: Bearer, X-API-Key. For others, please use `extra_headers` instead |
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.
Should we be having these as constants that people can pass in? It will prevent typos and give people examples of what to us
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.
@AutomatedTester, I updated it to use enum. This arg has a default value and is newly added, so I think it will not have a regression impact. Can you check if it is fine?
Signed-off-by: Viet Nguyen Duc <[email protected]>
if auth_type == "oauth" and self.token: | ||
return {"Authorization": f"OAuth {self.token}"} | ||
return {"Authorization": f"{AuthType.BASIC.value} {encoded_credentials}"} | ||
if self.auth_type is AuthType.BEARER and self.token: |
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.
I think this looks so much better!
User description
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
Continue of #13286
extra_headers
can be set via ClientConfiguser_agent
in request headers can be set via ClientConfigauth_type
supportsBearer, X-API-Key
. For other advanced methods, suggest usingextra_headers
instead.Motivation and Context
Types of changes
Checklist
PR Type
Enhancement, Tests
Description
ClientConfig
to support settinguser_agent
andextra_headers
, allowing more flexible HTTP request configurations.X-API-Key
, replacing the previousOAuth
option.ClientConfig
instead.user_agent
andextra_headers
intoRemoteConnection
, ensuring they are used in HTTP requests.Changes walkthrough 📝
client_config.py
Add user agent and extra headers to ClientConfig
py/selenium/webdriver/remote/client_config.py
user_agent
andextra_headers
parameters toClientConfig
.user_agent
andextra_headers
.X-API-Key
.remote_connection.py
Integrate user agent and headers in RemoteConnection
py/selenium/webdriver/remote/remote_connection.py
user_agent
andextra_headers
fromClientConfig
.remote_connection_tests.py
Add tests for user agent and extra headers in ClientConfig
py/test/unit/selenium/webdriver/remote/remote_connection_tests.py
user_agent
andextra_headers
inClientConfig
.