Skip to content
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

Use duck-typing to set a quasi-integer response status. #191

Closed
wants to merge 3 commits into from

Conversation

@perey
Copy link
Contributor

@perey perey commented Apr 7, 2015

This change checks for integer values when setting response.status by calling int() on the supplied value, instead of explicitly checking for a subclass of int.

This is helpful to me, because I'm writing code that wraps HTTP statuses in a class that defines __int__(). I'd like to be able to use WebOb together with this class without having to check whether the Response object accepted the status, or whether I need to call int() (or str() for that matter) and try again.

This change additionally removes the need for the "if ' ' not in value" check, which assumed value was interpretable by int() anyway and then performed the same string substitution as the setter for response.status_code.

@mmerickel
Copy link
Member

@mmerickel mmerickel commented Apr 7, 2015

Can you please fix the failing tests.

@bertjwregeer bertjwregeer added this to the Version 1.5 milestone Apr 7, 2015
try:
value += ' ' + status_reasons[int(value)]
except KeyError:
value += ' ' + status_generic_reasons[int(value) // 100]

This comment has been minimized.

@mmerickel

mmerickel Apr 13, 2015
Member

We are losing some validation here I think.

response.status = 'InvalidStatusCodeString' will pass here and be set when it should fail, right?

@bertjwregeer
Copy link
Member

@bertjwregeer bertjwregeer commented Apr 14, 2015

I pulled your changes, and rebased them on top of master.

bertjwregeer added a commit that referenced this pull request Apr 14, 2015
Rebased PR #191, but apparently missed part of the patch. This fixes
that.
@bertjwregeer
Copy link
Member

@bertjwregeer bertjwregeer commented Apr 14, 2015

Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants