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

Add HTTPX transport #370

Merged
merged 2 commits into from
Nov 26, 2022
Merged

Add HTTPX transport #370

merged 2 commits into from
Nov 26, 2022

Conversation

helderco
Copy link
Contributor

Fixes #154

This is an alternative to #340.

Notes

  • Both sync and async transports
  • File upload support (but no stream or AsyncGenerator)
  • Docs
  • Sync tests based on requests tests
  • Async tests based on aiohttp tests
  • The other transports log the query and result in info but I think it makes more sense for debug

Signed-off-by: Helder Correia

@codecov
Copy link

codecov bot commented Nov 26, 2022

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (481c8c6) compared to base (5e47f5f).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##            master      #370    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           24        25     +1     
  Lines         2215      2321   +106     
==========================================
+ Hits          2215      2321   +106     
Impacted Files Coverage Δ
gql/__version__.py 100.00% <100.00%> (ø)
gql/transport/aiohttp.py 100.00% <100.00%> (ø)
gql/transport/httpx.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@helderco
Copy link
Contributor Author

I can't build docs:

docs run-test: commands[0] | sphinx-build -b html -nEW docs docs/_build/html
Traceback (most recent call last):
  File "/Users/helder/Projects/gql/.tox/docs/bin/sphinx-build", line 5, in <module>
    from sphinx.cmd.build import main
  File "/Users/helder/Projects/gql/.tox/docs/lib/python3.8/site-packages/sphinx/cmd/build.py", line 25, in <module>
    from sphinx.application import Sphinx
  File "/Users/helder/Projects/gql/.tox/docs/lib/python3.8/site-packages/sphinx/application.py", line 43, in <module>
    from sphinx.registry import SphinxComponentRegistry
  File "/Users/helder/Projects/gql/.tox/docs/lib/python3.8/site-packages/sphinx/registry.py", line 24, in <module>
    from sphinx.builders import Builder
  File "/Users/helder/Projects/gql/.tox/docs/lib/python3.8/site-packages/sphinx/builders/__init__.py", line 26, in <module>
    from sphinx.util import import_object, logging, progress_message, rst, status_iterator
  File "/Users/helder/Projects/gql/.tox/docs/lib/python3.8/site-packages/sphinx/util/rst.py", line 21, in <module>
    from jinja2 import Environment, environmentfilter
ImportError: cannot import name 'environmentfilter' from 'jinja2' (/Users/helder/Projects/gql/.tox/docs/lib/python3.8/site-packages/jinja2/__init__.py)

Signed-off-by: Helder Correia <174525+helderco@users.noreply.github.com>
@leszekhanusz
Copy link
Collaborator

Thanks a lot for your complete PR!

@leszekhanusz
Copy link
Collaborator

I can't build docs:

docs run-test: commands[0] | sphinx-build -b html -nEW docs docs/_build/html
Traceback (most recent call last):
  File "/Users/helder/Projects/gql/.tox/docs/bin/sphinx-build", line 5, in <module>
    from sphinx.cmd.build import main
  File "/Users/helder/Projects/gql/.tox/docs/lib/python3.8/site-packages/sphinx/cmd/build.py", line 25, in <module>
    from sphinx.application import Sphinx
  File "/Users/helder/Projects/gql/.tox/docs/lib/python3.8/site-packages/sphinx/application.py", line 43, in <module>
    from sphinx.registry import SphinxComponentRegistry
  File "/Users/helder/Projects/gql/.tox/docs/lib/python3.8/site-packages/sphinx/registry.py", line 24, in <module>
    from sphinx.builders import Builder
  File "/Users/helder/Projects/gql/.tox/docs/lib/python3.8/site-packages/sphinx/builders/__init__.py", line 26, in <module>
    from sphinx.util import import_object, logging, progress_message, rst, status_iterator
  File "/Users/helder/Projects/gql/.tox/docs/lib/python3.8/site-packages/sphinx/util/rst.py", line 21, in <module>
    from jinja2 import Environment, environmentfilter
ImportError: cannot import name 'environmentfilter' from 'jinja2' (/Users/helder/Projects/gql/.tox/docs/lib/python3.8/site-packages/jinja2/__init__.py)

It's because of this issue.
As a workaround, you can run pip install jinja2==3.0 and it should work.
I'll upgrade sphinx in a separate PR to fix this cleanly.

@helderco
Copy link
Contributor Author

Thanks, I was able to run locally and fixes already sent!

I'll upgrade sphinx in a separate PR to fix this cleanly.

👍

Copy link
Collaborator

@leszekhanusz leszekhanusz left a comment

Choose a reason for hiding this comment

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

Please add the following changes:

  • add httpx dependencies in docs/intro.rst
  • add httpx link in docs/usage/file_upload.rst
  • add a file in docs/modules for httpx and add it to docs/modules/gql.rst This should fix the non-working ref in the doc

Please don't use force-push but instead push a normal commit so that we can see the changes more easily.

tests/test_httpx_online.py Outdated Show resolved Hide resolved
@helderco
Copy link
Contributor Author

Do I add the gql-with-httpx to conda instructions in intro.rst? Where are those created?

@helderco
Copy link
Contributor Author

Please don't use force-push but instead add a push a normal commit so that we can see the changes more easily.

Got it, but fyi, in Github you can see what changed with these buttons:

Screen Shot 2022-11-26 at 13 22 18

At dagger we amend a lot to keep the history clean.

@leszekhanusz
Copy link
Collaborator

Do I add the gql-with-httpx to conda instructions in intro.rst? Where are those created?

In another repository.
Maybe don't add it right now, I'll add it later with the next stable version of gql.

Signed-off-by: Helder Correia <174525+helderco@users.noreply.github.com>
@leszekhanusz
Copy link
Collaborator

Got it, but fyi, in Github you can see what changed with these buttons:

👍

At dagger we amend a lot to keep the history clean.

I'm not a fan of force-pushing except in very rare occurrences.
The commits here are squashed anyway when we merge so the main history will still stay clean 😄

@leszekhanusz leszekhanusz merged commit 0819418 into graphql-python:master Nov 26, 2022
@helderco helderco deleted the httpx branch November 26, 2022 14:40
@helderco
Copy link
Contributor Author

Thanks for the quick review and merge! 🎉

@leszekhanusz
Copy link
Collaborator

Thanks for the quick review and merge! tada

And thank you again for your help!

@prabhu
Copy link

prabhu commented Dec 16, 2022

Any ideas when this would be released?

@leszekhanusz
Copy link
Collaborator

@prabhu it's already released in a pre-release: v3.5.0b0

@prabhu
Copy link

prabhu commented Dec 16, 2022

Thanks @leszekhanusz. Will wait for the final release.

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

Successfully merging this pull request may close these issues.

HTTPX Transport
3 participants