-
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
Fix logging and new name for allowlist #27673
Conversation
API change check API changes are not detected in this pull request. |
class HttpLoggingPolicy(SansIOHTTPPolicy): | ||
@property | ||
def DEFAULT_HEADERS_WHITELIST(cls): | ||
return cls.DEFAULT_HEADERS_ALLOWLIST |
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.
Do we want to issue a deprecation warning if this is used? Or will that be too noisy for the time being?
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 it's too noisy, we don't have any reasons to remove it for now. Maybe if we feel it becomes a maintenance burden, we can add it later when it was not documented for months
@@ -1,5 +1,17 @@ | |||
# Release History | |||
|
|||
## 1.3.3 (Unreleased) |
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 change is a patch, not a minor?
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.
Is that really a new feature?
The few remaining red CI are unrelated, merging |
Fix #26331
Note: It's incredibly more difficult to hide an alias to a class property than it seems, as there is no class level
getattr
. But the metaclass option works fine, and is not read by Sphinx: