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 Response.url #699

Closed
wants to merge 1 commit into from
Closed

Add Response.url #699

wants to merge 1 commit into from

Conversation

natebosch
Copy link
Member

@natebosch natebosch commented May 25, 2022

Closes #321, closes #623, closes #692
Fixes #293

Add a Uri field to BaseResponse with the final, potentially redirected,
url for the content.

The field is nullable for backwards compatibility - TODO: consider if
this should be non-nullable from the start, or if it can be published
nullable first and become non-nullable (and the constructor arg
required) in the next breaking release.

This may break tests which use mocks but do not mock the field used to
read the URL.

Closes #321, closes #623, closes #692
Fixes #293

Add a Uri field to BaseResponse with the final, potentially redirected,
url for the content.

The field is nullable for backwards compatibility - TODO: consider if
this should be non-nullable from the start, or if it can be published
nullable first and become non-nullable (and the constructor arg
required) in the next breaking release.

This may break tests which use bocks but do not mock the field used to
read the URL.
@natebosch
Copy link
Member Author

This is going to need more validation and potentially a rollout plan.

@glassmonkey
Copy link

Thankyou !!

@glhvta
Copy link

glhvta commented Mar 2, 2023

Hello, could you please update on the status of this PR? We faced the issue in our project that getting the final redirected URL is not possible from http.Response. It would be great to have this functionality. Thanks!

@natebosch natebosch added the next-breaking-release Issues that are worth doing but need to wait for a breaking version bump label Mar 6, 2023
@natebosch
Copy link
Member Author

This is blocked on a breaking version change.

@henryssunday
Copy link

Any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next-breaking-release Issues that are worth doing but need to wait for a breaking version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get final redirected URL from response
4 participants