-
Notifications
You must be signed in to change notification settings - Fork 7
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
Modify logging module #43
Conversation
alluxiofs/core.py
Outdated
@@ -120,6 +116,17 @@ def __init__( | |||
if preload_path is not None: | |||
self.alluxio.load(preload_path) | |||
|
|||
if "log_level" in test_options: |
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.
think u should include in the options
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.
alluxiofs is an inner Python module
For each Python process, the end launch program should define to log level, and all the inner Python module inherit the logging configuration.
E.g. in our benchmark entry point, we should define the log level and log file there
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.
no I meant to say why not use the dict "options" ? its also user defined at time of instantiating alluxiofs instance right?
alluxiofs/core.py
Outdated
elif log_level == "WARN" or log_level == "WARNING": | ||
logger.setLevel(logging.WARN) | ||
else: | ||
logger.warning(f"Unsupported log level: {log_level}") |
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.
and lets set a default warning level to be ERROR (logging has ERROR level correct? )
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 usually don't have error, if it's error, the program should directly error out and failed
but they do have error log level
alluxiofs/core.py
Outdated
start_time = time.time() | ||
res = alluxio_impl(self, path, *args, **kwargs) | ||
self.logger.debug( | ||
f"Successfully called {alluxio_impl.__name__} against alluxio server with args ({args}), kwargs ({kwargs}) for {time.time() - start_time} ms" |
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.
this time.time() is in sec
@ChunxuTang @lucyge2022 PTAL thanks! |
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.
LGTM
No description provided.