-
Notifications
You must be signed in to change notification settings - Fork 3k
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
fix(ingestion): ensure source/sink reports are always logged #4592
fix(ingestion): ensure source/sink reports are always logged #4592
Conversation
minor updates to language
pipeline.run() | ||
logger.info("Finished metadata pipeline") | ||
except Exception as e: | ||
logger.error(f"Caught exception while running metadata ingestion: {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.
Let's log the error and rethrow so that the process doesn't exit 0
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.
Also, we don't want to capture misconfigurations by the user in the telemetry. So, re-throwing is the right thing to do 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.
I did not want to lose pipeline.pretty_print_summary
is the main reason I did not re-throw the exception. I pushed a change for non-zero return code. Let me know if that takes care of the concern.
Sometimes the report does not get printed which makes it hard to debug problems. This should help with that.
Checklist