-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Warn users of sslCommonName
deprecation.
#2704
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2704 +/- ##
========================================
Coverage 95.22% 95.23%
========================================
Files 60 60
Lines 12283 12290 +7
========================================
+ Hits 11697 11704 +7
Misses 586 586
Continue to review full report at Codecov.
|
66d6567
to
55848b5
Compare
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.
Just a quick note on testing and a minor nit. Otherwise, looks good.
botocore/client.py
Outdated
f'The {service_name} client is currently using a ' | ||
f'deprecated endpoint: {sslCommonName}. In the next ' | ||
f'minor version this will be moved to {hostname}. ' | ||
'See https://github.com/boto/botocore/issues/2376 ' |
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.
Perhaps it might be better if we create an issue that's a bit more explicit on the what/why surrounding this deprecation. It feels a little weird to link to a closed issue that doesn't really explain the deprecation beyond describing one of the affected services.
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.
Yeah that makes sense I'll make one
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.
f2cb691
to
cd1e8b4
Compare
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.
One last tweak and I think this is set!
d3534c8
to
e3ef3d9
Compare
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.
🚢
This somewhat addresses #2376. Since we changes to endpoints are coming soon, we aren't addressing the root of the problem at the moment, but sending a warning that it will be soon.