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

Import private httpx modules #209

Closed
wants to merge 2 commits into from
Closed

Import private httpx modules #209

wants to merge 2 commits into from

Conversation

suud
Copy link

@suud suud commented Mar 20, 2020

The public api of httpx has changed in version 0.12.1.

Fixes: #208

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:


  • You consent that the copyright of your pull request source code belongs to Authlib's author.

The public api of httpx has changed in version 0.12.1.

Fixes: #208
@phy25
Copy link

phy25 commented Mar 20, 2020

I think a try...catch ImportError... with attempts of different namespaces would be better?

The public api of httpx has changed in version 0.12.1.

Fixes: #208
@pdiwan
Copy link

pdiwan commented Mar 23, 2020

@lepture could you please get this fix merged? This is a blocking bug for me.

@lepture
Copy link
Owner

lepture commented Mar 23, 2020

@pdiwan you can use the old version of httpx. It shouldn't be a blocking issue.

@pdiwan
Copy link

pdiwan commented Mar 23, 2020

I could but just trying to avoid hardcoding versions now and later change when the fix is merged.

@@ -1,5 +1,8 @@
from httpx import URL
from httpx.content_streams import ByteStream
try:
from httpx._content_streams import ByteStream
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think it is a good idea to import from private modules. I need to ask @encode team to make it public.

Copy link
Contributor

Choose a reason for hiding this comment

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

So, it's not obvious to me that the rebuild_request function needs to be written in a way that uses private API here. Can you change that function instead? Perhaps like so...

def rebuild_request(request, url, headers, body):
    method = request.method
    headers = Headers(request.headers)
    headers.update(headers)
    return Request(method=method, url=url, headers=headers, body=body)

Copy link
Owner

Choose a reason for hiding this comment

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

@tomchristie unfortunately, there is no body parameter in Request.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me rephrase that. content=...

Copy link
Owner

Choose a reason for hiding this comment

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

@tomchristie there is no content either, the only available parameter would be data= (I think). But this rebuild_request is used in auth_flow, and it is requires_request_body = True. The request returned by rebuild_request is not read or aread yet, thus this request would not have .content property.

Copy link
Owner

Choose a reason for hiding this comment

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

@tomchristie it would also be helpful if we can add a property setter on Request:

    @content.setter
    def content(self, value: bytes) -> None:
        self._content = value
        self.stream = ByteStream(self._content)

Do you think it is ok to add this setter?

@lepture lepture closed this May 2, 2020
@lepture
Copy link
Owner

lepture commented May 2, 2020

Please pin httpx version.

try:
from httpx._dispatch.base import AsyncDispatcher
except ImportError:
from httpx.dispatch.base import AsyncDispatcher
Copy link
Owner

Choose a reason for hiding this comment

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

This module will be changed again in v0.13. That's why I'd like to keep our current implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

An alternative to using the private dispatch API here would be to implement the stubs as either ASGI or WSGI applications, and use Client(app=wsgi_app) or AsyncClient(app=asgi_app).

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.

incompatibility with new httpx version
5 participants