-
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
C4-352 Patch error info to FileProcessed #243
Conversation
willronchetti
commented
Oct 21, 2020
- When an error occurs in ingestion, patch the error string to the associated FileProcessed
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 see any blockers.
src/encoded/ingestion_listener.py
Outdated
@@ -669,6 +682,8 @@ def post_variants_and_variant_samples(self, parser, file_meta): | |||
success += 1 | |||
except Exception as e: # ANNOTATION spec validation error, recoverable | |||
log.error('Encountered exception posting variant at row %s: %s ' % (idx, e)) | |||
if self.first_error is None: | |||
self.first_error = str(e) |
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 would call this self.first_error_message
if it's only going to have the string.
src/encoded/ingestion_listener.py
Outdated
@@ -669,6 +682,8 @@ def post_variants_and_variant_samples(self, parser, file_meta): | |||
success += 1 | |||
except Exception as e: # ANNOTATION spec validation error, recoverable | |||
log.error('Encountered exception posting variant at row %s: %s ' % (idx, e)) | |||
if self.first_error is None: |
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 not self.first_error
be both simpler and more robust in case someone puts ""
there. I only go for is None
when the other falsey values would be possibly meaningful to mean something other than "missing".
variant_props = get_item_or_none(request, variant, 'Variant') | ||
variant_props = get_item_or_none(request, variant, 'Variant', frame='raw') | ||
if variant_props is None: | ||
raise RuntimeError('Got none for something that definitely exists') | ||
file_path = '%s/bamsnap/chr%s:%s.png' % ( # file = accession of associated VCF file |
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.
Hmm. Is .png
always the format? And the file
seems to be accession file path, a kind of base filename, is that right? A doc string here would help, I guess. :)