Skip to content

missing Response.ok method #1408

Closed
Closed

Description

@codenoid

resp.ok is cute

Activity

  1. lovelydinosaur commented on Dec 2, 2020

    @lovelydinosaur
    Member

    We've got a comment in the "requests compatibility" docs about this.

  2. StephenBrown2 commented on Dec 2, 2020

    @StephenBrown2
    Contributor
  3. serg06 commented on Mar 14, 2021

    @serg06

    if not resp.is_error just doesn't hit the same as if resp.ok 🙁

  4. Colin-b commented on Mar 14, 2021

    @Colin-b
    Contributor

    See #809

    I fail to see a use case were you would want to discard any errors (no logs, no treatment at all) and only perform action in case of successful response. This would be the only use case for a if response.ok and it seems like bad practice. Do you have such use case ?

  5. serg06 commented on Mar 14, 2021

    @serg06

    See #809

    I fail to see a use case were you would want to discard any errors (no logs, no treatment at all) and only perform action in case of successful response. This would be the only use case for a if response.ok and it seems like bad practice. Do you have such use case ?

    I don't, I'm just a sassy opinionated coder

  6. Colin-b commented on Mar 14, 2021

    @Colin-b
    Contributor

    See #809
    I fail to see a use case were you would want to discard any errors (no logs, no treatment at all) and only perform action in case of successful response. This would be the only use case for a if response.ok and it seems like bad practice. Do you have such use case ?

    I don't, I'm just a sassy opinionated coder

    So you don't need to use if not, you can replace your:

    import httpx
    
    response = httpx.get()
    if not response.is_error:
        handle_success()
    else:
        handle_error()

    by:

    import httpx
    
    response = httpx.get()
    if response.is_error:
        handle_error()
    else:
        handle_success()

    And then you have it, no negation involved :)

  7. serg06 commented on Mar 14, 2021

    @serg06

    And then you have it, no negation involved :)

    But then I'd be putting the (unlikely) error case before the (likely) success case!

  8. lovelydinosaur commented on Mar 14, 2021

    @lovelydinosaur
    Member

    if not resp.is_error just doesn't hit the same as if resp.ok 🙁

    Them's the breaks.

  9. locked and limited conversation to collaborators on Mar 14, 2021
  10. unlocked this conversation on Mar 22, 2021
  11. lovelydinosaur commented on Mar 22, 2021

    @lovelydinosaur
    Member

    So, the reason we don't do this is pretty clear.
    As a developer, what do you think response.ok means?

    • response.status_code == 200 A "200 OK" response
    • response.status_code >= 200 and response.status_code <= 299 Any 2xx response
    • not response.is_error Not a "4xx, 5xx" response

    It's just not at all clear. (Incidentally, in requests it means the last one of those.)

    I could perhaps see us having response.is_success, response.is_client_error and response.is_server_error properties., which are way more obviously 2xx, 4xx, and 5xx classes of response. But... note that would open up easy coding error classes like...

    response = httpx.get("https://www.example.com", allow_redirects=False)
    if response.is_success:
        ...  # Success condition
    else:
        ...  # Error condition

    Which doesn't do what it might look like it does on first sight.

    So... sure if not response.is_error ain't got as much sass as if response.ok, or even if response.is_success, but it'll help prevent coding errors.

    (I mean there's other options too. For instance, response.is_2xx, response.is_3xx, etc... but in terms of requests-parity I'm pretty convinced we don't want the misleading response.ok)

  12. locked and limited conversation to collaborators on Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions