-
Notifications
You must be signed in to change notification settings - Fork 9
Implemented the show-log command along with flag: -f, --filepath #17
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Shashank Tippanavar <[email protected]>
Implementation of show_log Signed-off-by: Shashank Tippanavar <[email protected]>
Update __init__.py to include option for --filepath flag Signed-off-by: Shashank Tippanavar <[email protected]>
Updated cli.py to include option for calling show-log Signed-off-by: Shashank Tippanavar <[email protected]>
Signed-off-by: Shashank Tippanavar <[email protected]>
Signed-off-by: Shashank Tippanavar <[email protected]>
Signed-off-by: Shashank Tippanavar <[email protected]>
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.
thank you, good work!
Leaving these comments for gsoc reviews. Not approving since the PR is just for evaluation task.
|
||
async def show_log(self, filepath: bool = False): | ||
home_dir = os.path.expanduser("~") | ||
target_dir = os.path.join(home_dir, ".local", "share", "ceph-devstack", "archive") |
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.
~/.local/share/ceph-devstack
is the default directory where archives are stored but it can be configureable with --data-dir
so it's a good idea to not hardcode the archive path here
@@ -226,3 +227,68 @@ async def wait(self, container_name: str): | |||
return await object.wait() | |||
logger.error(f"Could not find container {container_name}") | |||
return 1 | |||
|
|||
async def show_log(self, filepath: bool = False): |
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.
Nit: boolean variables can be named something like is_filepath
for readability
elif filepath: | ||
logger.info(f"{log_file_path}") | ||
return 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.
elif filepath: | |
logger.info(f"{log_file_path}") | |
return 0 | |
elif os.path.exists(log_file_path) and os.path.isfile(log_file_path) and filepath: | |
logger.info(f"{log_file_path}") | |
return 0 |
otherwise it'll print the teuthology.log file path even if job directory exists but teuthology.log file does not exist
elif filepath: | ||
logger.info(f"{log_file_path}") |
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.
elif filepath: | |
logger.info(f"{log_file_path}") | |
elif os.path.exists(log_file_path) and os.path.isfile(log_file_path) and filepath: | |
logger.info(f"{log_file_path}") |
same comment as above
except Exception as e: | ||
logger.error(f"Error extracting timestamp from {dir_name}: {e}") | ||
return None | ||
run_directories_list.sort(key=lambda dir: extract_timestamp(dir), reverse=True) |
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 is nice!
The show-log command displays the contents of the most recent teuthology.log file. The optional -f or --filepath flag can be used to display only the filepath of the latest teuthology.log file instead of its contents.
Usage:
ceph-devstack show-log -f