Add PendingDeprecationWarning to internal requests methods#1625
Add PendingDeprecationWarning to internal requests methods#1625jamesls merged 2 commits intoboto:developfrom
Conversation
This produces no output yet unless you explicitly turn on warnings. In a future release we'll have this emit a warning by default. I opted to put this in the high level `botocore.vendored.requests.*` methods because that's the more common usage of requests and I wanted better error messages. Moving it to the `Session.request` method would mean we couldn't give as helpful of an error message.
Codecov Report
@@ Coverage Diff @@
## develop #1625 +/- ##
========================================
Coverage 91.99% 91.99%
========================================
Files 51 51
Lines 9335 9335
========================================
Hits 8588 8588
Misses 747 747Continue to review full report at Codecov.
|
botocore/vendored/requests/api.py
Outdated
| """ | ||
| warnings.warn( | ||
| _WARNING_MSG.format(name=method), | ||
| PendingDeprecationWarning |
There was a problem hiding this comment.
This should be DeprecationWarning since it's already deprecated. Though I'm not sure deprecated is right at all since this was never meant to be used in the first place. Might be better to stick with UserWarning since that will be shown by default.
There was a problem hiding this comment.
Hmm, I think either PendingDeprecationWarning or DeprecationWarning is appropriate.
I'm -1 on labeling this as a UserWarning just to get it to show up by default, as I think it would go against what users would expect of a deprecation warning.
There was a problem hiding this comment.
Let's switch it to DeprecationWarning for now. I think we'll eventually convert this to a warning that shows up by default, but I want to put something in place now and let it sit for a while before we start printing warnings to stderr.
joguSD
left a comment
joguSD
left a comment
There was a problem hiding this comment.
Do you think it would be possible to put this in the Session constructor instead? That way if they're going through the shortcuts or managing the Session object themselves they'll still get warned.
botocore/vendored/requests/api.py
Outdated
| """ | ||
| warnings.warn( | ||
| _WARNING_MSG.format(name=method), | ||
| PendingDeprecationWarning |
There was a problem hiding this comment.
Hmm, I think either PendingDeprecationWarning or DeprecationWarning is appropriate.
I'm -1 on labeling this as a UserWarning just to get it to show up by default, as I think it would go against what users would expect of a deprecation warning.
|
@joguSD so my main concern was with trying to get a decent error message. Putting in the |
PR #1625 * vendored-requests-warning: Change PendingDeprecationWarning to DeprecationWarning Add PendingDeprecationWarning to internal requests methods
4 participants
This produces no output yet unless you explicitly turn on
warnings. In a future release we'll have this emit a warning
by default.
I opted to put this in the high level
botocore.vendored.requests.*methods because that's the more common usage of requests and I wanted
better error messages. Moving it to the
Session.requestmethodwould mean we couldn't give as helpful of an error message.
Testing