Skip to content

Cleanup Logging for python functions#2847

Merged
jerrypeng merged 4 commits intoapache:masterfrom
srkukarni:python_cleanuplogs
Oct 26, 2018
Merged

Cleanup Logging for python functions#2847
jerrypeng merged 4 commits intoapache:masterfrom
srkukarni:python_cleanuplogs

Conversation

@srkukarni
Copy link
Copy Markdown
Contributor

Motivation

Move a lot of logging from .info to .debug.
Also cleaned up the import logging exceptions.

Modifications

Describe the modifications you've done.

Result

After your change, what will change.

@srkukarni srkukarni requested a review from jerrypeng October 25, 2018 19:16
@srkukarni
Copy link
Copy Markdown
Contributor Author

rerun integration tests

Log.info("Import failed class_name %s from path %s" % (class_name, from_path))
Log.info(e, exc_info=True)
return None
mod = importlib.import_module(class_name)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can an exception be thrown here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This causes cascading changes to all users of this function(of which there are many). Maybe that cleanup belongs to another change?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

didn't this throw an exception before and we catch it and when to do print. If we don't do the try and except, wouldn't it crash the function instance?

Copy link
Copy Markdown
Contributor Author

@srkukarni srkukarni Oct 25, 2018

Choose a reason for hiding this comment

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

Sorry, I mistook the part of the code.
So this function is only called from the functions above(import_class). This function was prev returning None in exceptions and this pr makes it throw an exception which the caller catches and logs about that exception

@srkukarni
Copy link
Copy Markdown
Contributor Author

rerun integration tests

@jerrypeng jerrypeng merged commit fa212bc into apache:master Oct 26, 2018
@srkukarni srkukarni deleted the python_cleanuplogs branch October 26, 2018 17:21
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