Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make raise_for_status chainable #2776

Merged
merged 9 commits into from
Aug 1, 2023

Conversation

trim21
Copy link
Contributor

@trim21 trim21 commented Jul 15, 2023

Summary

make it possible to write code like r = httpx.get('...').raise_for_status().json()

Refs #2775

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@trim21 trim21 force-pushed the raise_for_status-chain branch from a2f8dbe to 00c6795 Compare July 15, 2023 08:31
@trim21 trim21 marked this pull request as ready for review July 15, 2023 08:37
@tomchristie tomchristie requested a review from a team July 31, 2023 11:48
@tomchristie tomchristie changed the title [WIP]: make raise_for_status chainable Make raise_for_status chainable Jul 31, 2023
Copy link
Member

@abersheeran abersheeran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. It's a small change, but the experience will be much better.

@tomchristie
Copy link
Member

Okay great, so...

  • CHANGELOG.
  • Do we want to mention this usage style in the docstring? In the docs?

@zanieb
Copy link
Contributor

zanieb commented Jul 31, 2023

I think this usage should be mentioned in the docstring and prose documentation.

@michaeloliverx
Copy link
Member

Really nice DX improvement here, I see myself using this a lot!

@trim21
Copy link
Contributor Author

trim21 commented Jul 31, 2023

document updated
changelog added

Copy link
Member

@tomchristie tomchristie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Some minor suggestions...

(Other could also be considered)

docs/quickstart.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
docs/quickstart.md Outdated Show resolved Hide resolved
trim21 and others added 3 commits August 1, 2023 17:16
Co-authored-by: Tom Christie <tom@tomchristie.com>
Co-authored-by: Tom Christie <tom@tomchristie.com>
Co-authored-by: Tom Christie <tom@tomchristie.com>
@tomchristie tomchristie merged commit 9415af6 into encode:master Aug 1, 2023
@trim21 trim21 deleted the raise_for_status-chain branch August 1, 2023 09:59
@trim21
Copy link
Contributor Author

trim21 commented Aug 1, 2023

is there any ETA on next release?

@tomchristie
Copy link
Member

If you're up for helping drive a release pull request, then we could fix up a minor point release whenever really.

@karpetrosyan
Copy link
Member

If you're up for helping drive a release pull request, then we could fix up a minor point release whenever really.

Can we include encode/httpcore#745 in the next httpx release?

First, we should release httpcore with that pull request and then release httpx

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants