-
Notifications
You must be signed in to change notification settings - Fork 0
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 skipping mechanism for list of studies #360
Conversation
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.
Thanks @KateSakharova ! Couple of tiny suggestions inline.
Please can you also change the PR base branch to develop
? Thanks!
emgapi/metagenomics_exchange.py
Outdated
pass | ||
raise http_error | ||
return response | ||
except ValueError: # Catch JSON decoding errors |
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.
Wouldn't this raise an requests.exceptions.JSONDecodeError
rather than a ValueError
🤔 ?
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.
you are right, one is a subclass of another. Fixed!
@@ -131,6 +131,9 @@ def process_to_index_and_update_records(self, analyses_to_index_and_update): | |||
mgya=annotation_job.accession, | |||
sequence_accession=sequence_accession, | |||
) | |||
if not response: | |||
logging.info(f"Error occured {annotation_job}") |
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.
logging.info(f"Error occured {annotation_job}") | |
logging.warning(f"Error occurred {annotation_job}") |
I suppose it is bad enough for a warning :)
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.
Looks good to me.. other that Sandy's comments.
If ME post request doesn't work - it raised error and fail for whole run. This fix will skip that study raising HTTP error and continue with next study in list