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

ignore dataset description if katsu down #80

Merged
merged 1 commit into from
Apr 25, 2024
Merged

Conversation

gsfk
Copy link
Member

@gsfk gsfk commented Apr 17, 2024

Corrects an issue where /service-info would return an invalid response when katsu is down (it would return an error response in beacon format, but that doesn't match the spec for service-info). Also allows the root endpoint / to return service details even when katsu is down, instead of an error response.

Some info endpoints will still return errors when katsu is down (/info, /overview and others) but this is expected.

@gsfk gsfk marked this pull request as ready for review April 18, 2024 11:48
@gsfk gsfk requested a review from davidlougheed April 18, 2024 11:48
try:
response = katsu_get(endpoint, id, query="format=phenopackets")
except APIException:
return {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a log warning message or something if this happens, with the APIException contents?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or is this handled in katsu_get

Copy link
Member Author

@gsfk gsfk Apr 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this will be logged already

Copy link
Member

@davidlougheed davidlougheed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@gsfk gsfk merged commit b82484d into master Apr 25, 2024
2 checks passed
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.

2 participants