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

[requests_toolbelt.session] Prepared requests do not have the complete url #315

Closed
mbBRCM opened this issue Sep 25, 2021 · 9 comments · Fixed by #317
Closed

[requests_toolbelt.session] Prepared requests do not have the complete url #315

mbBRCM opened this issue Sep 25, 2021 · 9 comments · Fixed by #317

Comments

@mbBRCM
Copy link
Contributor

mbBRCM commented Sep 25, 2021

This is working:

response = self.session.put(endpoint, headers=headers, data=string)

But this is not:

request = Request("PUT", endpoint, headers=headers, data=data)
prepped = session.prepare_request(request)
response = session.send(prepped)

Does requests_toolbelt have a way to prepare requests with the base url?

@sigmavirus24
Copy link
Collaborator

No, it doesn't

@mbBRCM
Copy link
Contributor Author

mbBRCM commented Oct 7, 2021

@sigmavirus24 Is this something that could be added?

@sigmavirus24
Copy link
Collaborator

Yes, it is

@mbBRCM
Copy link
Contributor Author

mbBRCM commented Oct 7, 2021

@sigmavirus24 Is it ok if I give it a try?

@sigmavirus24
Copy link
Collaborator

Absolutely!

@mbBRCM
Copy link
Contributor Author

mbBRCM commented Oct 11, 2021

What do you think of this solution?

    def prepare_request(self, request, *args, **kwargs):
        """Prepare the request after generating the complete URL."""
        url = self.create_url(request.url)
        new_request = copy(request)
        new_request.url = url
        return super(BaseUrlSession, self).prepare_request(
            new_request, *args, **kwargs
        )

The copy is so we do not mutate the original Request object

@sigmavirus24
Copy link
Collaborator

I don't believe a copy is the right thing to do. Users can have file objects and such and pretending to copy will not preserve anything.

@mbBRCM
Copy link
Contributor Author

mbBRCM commented Oct 12, 2021

Oh, ok - so is it fine to just substitute the complete url in the user's Request like this?

    def prepare_request(self, request, *args, **kwargs):
        """Prepare the request after generating the complete URL."""
        request.url = self.create_url(request.url)
        return super(BaseUrlSession, self).prepare_request(
            request, *args, **kwargs
        )

@sigmavirus24
Copy link
Collaborator

Yup. That's how I'd expect it to work because all too often those requests can't be reused

mbBRCM added a commit to mbBRCM/toolbelt that referenced this issue Oct 13, 2021
We now override the `prepare_request` method of the Session
to generate a complete URL from the session's base URL
and a partial resource name. The request is then prepared with
the complete URL.

  >>> from requests import Request
  >>> from requests_toolbelt import sessions
  >>> s = sessions.BaseUrlSession(
  ...     base_url='https://example.com/resource/')
  >>> request = Request(method='GET', url='sub-resource/')
  >>> prepared_request = s.prepare_request(request)
  >>> r = s.send(prepared_request)
  >>> print(r.request.url)
  https://example.com/resource/sub-resource

Closes requests#315.
mbBRCM added a commit to mbBRCM/toolbelt that referenced this issue Oct 26, 2021
We now override the `prepare_request` method of the Session
to generate a complete URL from the session's base URL
and a partial resource name. The request is then prepared with
the complete URL.

  >>> from requests import Request
  >>> from requests_toolbelt import sessions
  >>> s = sessions.BaseUrlSession(
  ...     base_url='https://example.com/resource/')
  >>> request = Request(method='GET', url='sub-resource/')
  >>> prepared_request = s.prepare_request(request)
  >>> r = s.send(prepared_request)
  >>> print(r.request.url)
  https://example.com/resource/sub-resource

Closes requests#315.
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 a pull request may close this issue.

2 participants