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

Include path if the base url is generated from the request URL. #1602

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

JPBergsma
Copy link
Contributor

@JPBergsma JPBergsma commented Apr 13, 2023

This PR fixes a bug in get_base_url.
If the base URL is not specified in the config file, the function get_base_url extracts the base URL from the request URL.
The current function, however, only returns the scheme (e.g. https) and netloc(e.g. www.example.com) part of the base URL.
A base URL may however also contain a root path (e.g. /optimade).
This patch adds the root path to the base URL.

@JPBergsma JPBergsma marked this pull request as draft April 14, 2023 09:45
@JPBergsma
Copy link
Contributor Author

JPBergsma commented Apr 14, 2023

On second thought, I don't think this is needed as it is also possible to set the root_path via a config option.
The root path is not automatically added to the base URL, so a patch for the get_base_url function is still needed.

@JPBergsma JPBergsma closed this Apr 14, 2023
@JPBergsma JPBergsma reopened this Apr 18, 2023
@codecov
Copy link

codecov bot commented Apr 18, 2023

Codecov Report

Merging #1602 (d0c76cd) into master (8b3e146) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1602   +/-   ##
=======================================
  Coverage   91.04%   91.05%           
=======================================
  Files          74       74           
  Lines        4578     4583    +5     
=======================================
+ Hits         4168     4173    +5     
  Misses        410      410           
Flag Coverage Δ
project 91.05% <100.00%> (+<0.01%) ⬆️
validator 90.94% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
optimade/server/routers/utils.py 96.09% <100.00%> (+0.15%) ⬆️

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

@JPBergsma
Copy link
Contributor Author

I just noticed that this PR has a lot in common with PR #663

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.

None yet

1 participant