[shin4141]

What this fixes

If destination directory creation fails in move_log_file, the original file handler has already been closed and removed. A caller that catches the exception can emit a warning, but that warning is missing from the original log.

Create the destination directory before closing or removing the original handler, so a warning logged after directory creation fails is still written to the original log. Logging also remains usable for a later relocation retry.

Why it matters

Keeping failure diagnostics in the original log avoids an extra reproduction just to recover why relocation failed.

The current FOSSLight Scanner path creates the destination directory before calling move_log_file, so the impact there is limited. This fix primarily protects callers that use the Util API directly.

Review scope

• The implementation change is only one moved line: os.makedirs(...) runs before detaching the original file handler.
• Two files change: src/fosslight_util/set_log.py and tests/test_set_log.py.
• Four regression cases cover a destination parent that is a regular file, denied directory creation, existing move-failure recovery, and successful relocation. Failure cases check that subsequent warnings reach the original log and that retry preserves the log messages. Handler count, level, formatting, and UTF-8 messages are also checked.

Evidence

Verified in Cloud on Linux / Python 3.12.14 against upstream main c20cc2db02bc76849dd009fd4b0cb639169fdedb:

• Before the implementation change: 2 failed / 2 passed with the four regression cases. Both directory preparation failures lost the warning emitted after the exception.
• After the implementation change: regression 4 PASS / related 131 PASS / lint PASS.

python -m pytest -q tests/test_set_log.py
4 passed

python -m pytest -q tests --ignore=tests/legacy --ignore=tests/test_download.py -k 'not test_download_packagist_live'
131 passed, 2 deselected, 4 warnings

python -m flake8 src tests/test_set_log.py
PASS

The four related-suite warnings come from existing CycloneDX fixtures whose root component has no declared dependencies.

Only Linux / Python 3.12 was verified. External download integration tests (tests/test_download.py and the two live Packagist cases), legacy tests, other Python versions, and other platforms were not verified. The complete tox -e release CI workflow has not been run locally.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

• Configuration used: Repository: fosslight/fosslight_util/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 516be3e6-aa03-4a88-9597-f9b31e889d10

📥 Commits

Reviewing files that changed from the base of the PR and between 69d4ce3 and 1864746.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: fosslight/fosslight_util/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: d3ffa53f-3d60-445d-ae67-4e8b458fd4b0

📥 Commits

Reviewing files that changed from the base of the PR and between c20cc2d and 69d4ce3.

📒 Files selected for processing (2)

• src/fosslight_util/set_log.py
• tests/test_set_log.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

move_log_file creates the destination directory before inspecting file handlers. Tests cover logging continuity after relocation failures and handler settings after successful relocation.

Changes

Logging File Relocation

Layer / File(s)	Summary
Relocation ordering and behavior src/fosslight_util/set_log.py, tests/test_set_log.py	move_log_file creates the destination directory before searching for a matching file handler. Tests cover three failure cases, retry behavior, successful relocation, logged messages, and preserved handler settings.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 69d4c

Logging stays usable and warnings reach the original log file when destination setup fails, so a relocation can be retried. Regression tests cover failure and success cases. No merge-blocking risk is evident.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the logging fix: preserving the active handler when destination setup fails.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[soimkim]

@shin4141, Thanks for the contribution.

I removed the test changes because they validate implementation details rather than the API contract. Given the current FOSSLight Scanner usage, these additional tests do not provide sufficient value.