-
Notifications
You must be signed in to change notification settings - Fork 42
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
Update base URLs #155
Merged
CasperWA
merged 4 commits into
Materials-Consortia:master
from
CasperWA:fix_122_base_urls
Feb 3, 2020
Merged
Update base URLs #155
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, what exactly would be excepting in the
try
block?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah ... not much at this moment in time - but I hope to implement the OPTiMaDe warnings similarly to Python
Warning
s, which are a sub-class ofException
s. I.e., at some point again (hopefully soon) there will be raised something, which can be caught here and relayed as an OPTiMaDewarning
... But at the moment... yeah, not much.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite follow - the
except
block here would only be activated if there was a programming error in thetry
block, i.e. in the preparation of the error response object.In this case, one should not raise a Warning but an Error (since something would have gone seriously wrong).
The only difference I see between the response in
try
and inexcept
is that the one inexcept
does not add all the optimade stuff.Perhaps one can simply remove the try/except here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
except
block would be activated if the utility functionmeta_values
raises, for one reason or another. Withinmeta_values
is where themeta
->query
->representation
is created and returned, which is why i originally created theoptional_base_urls
function - to return all 6 different path prefixes so they could be removed from theurlparse
d path.I don't know if this makes sense ... but if it should happen that the URL path of the server, for any reason, does not have the prefixes we set for it (e.g.,
/optimade/v1
, which actually happens for theTestClient
used to test the server) it would in my original commit raise aHTTPException
. I tried to fix this in the test setup, but it eluded me, so instead I chose to backtrack, remove theHTTPException
raised inmeta_values
and simply keep thetry
andexcept
in this part - although it's technically not necessary (unlessmeta_values
should fail due to other unknown reasons).