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

Add ConnectionError handling for MLFlow logger #236

Merged
merged 1 commit into from Feb 6, 2020

Conversation

tanaysoni
Copy link
Contributor

This PR adds error handling in the MLFlow Logger

  • raise error and display error message(with guide to use MLFLow in local mode)when a remote MLFLow instance is unreachable
  • catch exceptions when a MLFlow instance is unreachable when logging metrics, params, or artifacts. This prevents training from crashing when a MLFLow server is momentarily unavailable.

Copy link
Contributor

@Timoeller Timoeller left a comment

Choose a reason for hiding this comment

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

The method looks totally fine.
Could you add the exceptions to the other places where the logger is used e.g. in farm/experiment.py please?

@tanaysoni
Copy link
Contributor Author

tanaysoni commented Feb 5, 2020

@Timoeller At other place like farm/experiment and the examples, the logging for MLFlow runs is done via the MLFlowLogger class, so the exception handling is covered. We should not need additional try catch blocks.

Copy link
Contributor

@Timoeller Timoeller left a comment

Choose a reason for hiding this comment

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

Ah, you are totally right. Approved.
I will better not review code from my mobile next time...

@tanaysoni tanaysoni merged commit caf7fdb into master Feb 6, 2020
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.

None yet

2 participants