Skip to content

Conversation

@plypaul
Copy link
Contributor

@plypaul plypaul commented Nov 10, 2025

As per title.

@cla-bot cla-bot bot added the cla:yes label Nov 10, 2025
@plypaul plypaul marked this pull request as ready for review November 10, 2025 18:45
@plypaul plypaul requested a review from a team as a code owner November 10, 2025 18:45
Copy link
Contributor

@tlento tlento left a comment

Choose a reason for hiding this comment

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

It'd be good to clean up the magic string before merge, or if that's an annoying thing requiring package updates and deployment then a follow up is fine.

)

# Check that log messages are written as spected to the log file.
log_file_path = tutorial_directory.joinpath("logs", CLIConfiguration.LOG_FILE_NAME)
Copy link
Contributor

Choose a reason for hiding this comment

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

"logs" is a magic string. Is there no constant for that path location name?

plypaul added a commit that referenced this pull request Dec 18, 2025
The `mf` CLI was not logging `INFO` messages to the log file. This was
traced to an imported module setting up the log configuration and
`logging.basicConfig` becoming a no-op if handlers were already added to
the root logger.

This PR updates the CLI's logging setup method to use
`logging.basicConfig(..., force=True)` to address the no-op case and
cleans up how the stream handler and the log file handler are set up.

A test is added separately
#1934 due to a required PR in
the middle.
@plypaul plypaul force-pushed the p/fix_cli_logging__02 branch from 1238a0b to 7ff4a4d Compare January 14, 2026 16:11
@plypaul plypaul force-pushed the p/fix_cli_logging__03 branch 2 times, most recently from a3a22e2 to 05c0a5b Compare January 14, 2026 16:55
@plypaul plypaul force-pushed the p/fix_cli_logging__02 branch from 38ef62a to 7ff4a4d Compare January 14, 2026 17:04
@plypaul plypaul force-pushed the p/fix_cli_logging__03 branch from 05c0a5b to fd4886e Compare January 14, 2026 17:04
@plypaul plypaul force-pushed the p/fix_cli_logging__03 branch from fd4886e to ddb0a92 Compare January 14, 2026 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants