Skip to content

[CLI] Make per-run log files unique by appending timestamp + open with mode="w" (fixes #984) #83

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.

Sign up for GitHub

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

Open
abhisheksingh-esp wants to merge 1 commit into
base: v2.15-cli-develop
Choose a base branch
from
+13 −3
Open

[CLI] Make per-run log files unique by appending timestamp + open with mode="w" (fixes #984) #83

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions th_cli/test_run/logging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
import datetime
import os

from loguru import logger
Expand All @@ -27,13 +28,22 @@
PYTHON_TEST_LEVEL = "PYTHON_TEST"
logger.level(PYTHON_TEST_LEVEL, no=22, icon="🐍", color="<cyan>")

_LOG_TIMESTAMP_FORMAT = "%Y-%m-%d-%H-%M-%S"

def configure_logger_for_run(title: str) -> str:
# Reset (Remove all sinks from logger)
logger.remove()

log_path = os.path.join(config.log_config.output_log_path, f"test_run_{title}.log")

logger.add(log_path, enqueue=True, format=config.log_config.format)
# Replace colons so the default timestamp-style title does not produce
# filenames that are invalid on Windows (e.g. when logs are shared across
# systems for triage).
safe_title = title.replace(":", "-")
timestamp = datetime.datetime.now().strftime(_LOG_TIMESTAMP_FORMAT)
Copy link
Copy Markdown
Contributor

@antonio-amjr antonio-amjr May 14, 2026

Choose a reason for hiding this comment

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

Can we use the .isoformat() for the timestamp?
It will have the appropriate format (YYYY-MM-DDTHH:MM:SS.ffffff) and we would use a standard approach.

Suggested change
timestamp = datetime.datetime.now().strftime(_LOG_TIMESTAMP_FORMAT)
timestamp = datetime.datetime.now().isoformat()

Let me know what you think and if that works.

Copy link
Copy Markdown
Author

@abhisheksingh-esp abhisheksingh-esp May 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Agreed — using .isoformat() makes sense here. It already provides the correct standardized format (YYYY-MM-DDTHH:MM:SS.ffffff) and keeps the implementation cleaner and more consistent.

Only one catch: isoformat() produces timestamps containing :, which makes the filename invalid on Windows (and some network/USB filesystems). This could create issues if the log file is copied to or opened on a Windows machine.

Copy link
Copy Markdown
Contributor

@antonio-amjr antonio-amjr May 15, 2026

Choose a reason for hiding this comment

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

Good catch about Windows systems.

Maybe that's not the best option.
Or maybe we could replace all : for - from the isoformat's output.

In that case, please feel free to continue with your solution if that would be a pain in Windows machines. Since my team is not using, sometimes those details may slip.

Copy link
Copy Markdown
Contributor

@antonio-amjr antonio-amjr May 18, 2026

Choose a reason for hiding this comment

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

Let me know which approach do you think would be better for all systems in this case

Copy link
Copy Markdown
Author

@abhisheksingh-esp abhisheksingh-esp May 19, 2026

Choose a reason for hiding this comment

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

Thanks. Let's go with isoformat(timespec="seconds").replace(":", "-") — gives us 2026-05-19T12-47-33. It keeps the ISO-style format we wanted, drops the noisy microseconds, and is safe on Windows, FAT/exFAT, and SMB shares (so users copying logs to a Windows machine for triage won't hit "invalid filename" errors).

log_path = os.path.join(
config.log_config.output_log_path,
f"test_run_{safe_title}_{timestamp}.log",
)
Comment thread
abhisheksingh-esp marked this conversation as resolved.

logger.add(log_path, enqueue=True, format=config.log_config.format, mode="w")

return log_path