Skip to content

Conversation

@MightyShashank
Copy link

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

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]>
@VallariAg VallariAg self-requested a review April 2, 2025 17:04
Copy link
Member

@VallariAg VallariAg left a 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")
Copy link
Member

@VallariAg VallariAg Apr 9, 2025

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

logger.error(f"Could not find container {container_name}")
return 1

async def show_log(self, filepath: bool = False):
Copy link
Member

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

Comment on lines +269 to +271
elif filepath:
logger.info(f"{log_file_path}")
return 0
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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

Comment on lines +285 to +286
elif filepath:
logger.info(f"{log_file_path}")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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)
Copy link
Member

Choose a reason for hiding this comment

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

this is nice!

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