-
-
Notifications
You must be signed in to change notification settings - Fork 937
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
feat (status_codes): update HTTP status constants wrt RFC 9110 #2330
base: master
Are you sure you want to change the base?
Conversation
refactor(response): replace falcon.HTTPPayloadTooLarge with falcon.HTTPContentTooLarge Replace the usage of falcon.HTTPPayloadTooLarge with falcon.HTTPContentTooLarge in the codebase. This change aligns with the recent renaming of the class in the Falcon library. The new class name better reflects the purpose of the error, which is to indicate that the content of the request is too large for the server to process. Closes falconry#2322
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, left a comment and a question for other maintainers
falcon/__init__.py
Outdated
@@ -363,7 +363,7 @@ | |||
from falcon.errors import HTTPNotAcceptable | |||
from falcon.errors import HTTPNotFound | |||
from falcon.errors import HTTPNotImplemented | |||
from falcon.errors import HTTPPayloadTooLarge | |||
from falcon.errors import HTTPContentTooLarge |
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.
@vytas7 do you think we should keep the old class as deprecated?
If so the new one should be a subclass of HTTPPayloadTooLarge
I guess.
Honestly not sure.
Edit: looking at the changelog, a similar thing was done tor 1.4->2.0. let's use the same approach!
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 definitely think we ought to keep old classes as deprecated, even if 4.0 is a major release, since we have not given the users any deprecation window.
Can you add a newsfragment? It could read something similar to the 2.0 changelog line |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2330 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 63 63
Lines 7604 7606 +2
Branches 1285 1285
=========================================
+ Hits 7604 7606 +2 ☔ View full report in Codecov by Sentry. |
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 this improvement!
We need to address a couple of issues before merging this (commented inline).
@@ -952,8 +952,8 @@ def __init__( | |||
) | |||
|
|||
|
|||
class HTTPPayloadTooLarge(HTTPError): | |||
"""413 Payload Too Large. | |||
class HTTPContentTooLarge(HTTPError): |
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.
We must keep the old name for compatibility. Even though 4.0 is a major release that affords breaking changes, we must allow for a longer deprecation window for the publicly documented stuff.
refactor(response): replace falcon.HTTPPayloadTooLarge with falcon.HTTPContentTooLarge
Closes #2276
Summary of Changes
Replace the usage of falcon.HTTPPayloadTooLarge with falcon.HTTPContentTooLarge in the codebase. This change aligns with the recent renaming of the class in the Falcon library. The new class name better reflects the purpose of the error, which is to indicate that the content of the request is too large for the server to process.
Related Issues
#2276
Pull Request Checklist
This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once; it will save you a few review cycles!
If an item doesn't apply to your pull request, check it anyway to make it apparent that there's nothing to do.
docs/
.docs/
.versionadded
,versionchanged
, ordeprecated
directives.docs/_newsfragments/
, with the file name format{issue_number}.{fragment_type}.rst
. (Runtowncrier --draft
to ensure it renders correctly.)If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!
PR template inspired by the attrs project.