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

[23.0] Fix incorrect ASGI request host #16574

Merged

Conversation

davelopez
Copy link
Contributor

Fixes #16454

I'm sorry, I think I mistakenly used the wrong host. It was returning the client host but I believe it should have been the application host instead 🤦‍♂️

This was introduced in 22.01 should I target that branch instead?

How to test the changes?

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@davelopez davelopez changed the base branch from dev to release_23.0 August 18, 2023 09:43
@github-actions github-actions bot added this to the 23.2 milestone Aug 18, 2023
@mvdbeek
Copy link
Member

mvdbeek commented Aug 18, 2023

Let's let the tests run here, but probably a good idea to backport

@mvdbeek
Copy link
Member

mvdbeek commented Aug 18, 2023

I think baseURL might've looked different in older starlette versions, now that I think of it

@github-actions github-actions bot modified the milestones: 23.2, 23.0 Aug 18, 2023
@davelopez
Copy link
Contributor Author

Thanks, Marius, I'll create the backport for 22.01

@mvdbeek mvdbeek merged commit b44d8e2 into galaxyproject:release_23.0 Aug 18, 2023
43 of 44 checks passed
@davelopez davelopez deleted the 23.0_fix_asgi_request_host branch August 19, 2023 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configuration API returns unexpected values for by_host option
2 participants