Skip to content

Log ci container#655

Merged
guarin merged 9 commits intomainfrom
trn-1839-log-ci-container
Mar 23, 2026
Merged

Log ci container#655
guarin merged 9 commits intomainfrom
trn-1839-log-ci-container

Conversation

@guarin
Copy link
Copy Markdown
Contributor

@guarin guarin commented Mar 20, 2026

What has changed and why?

  • Logs whether LightlyTrain is running in CI or a container

How has it been tested?

  • Unit tests

Did you update CHANGELOG.md?

  • Yes
  • Not needed (internal change)

Did you update the documentation?

  • Yes
  • Not needed (internal change without effects for user)

Copilot AI review requested due to automatic review settings March 20, 2026 12:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds lightweight runtime environment signals to LightlyTrain event analytics so events can be tagged as running in CI and/or inside a container.

Changes:

  • Extend _get_system_info() to include is_ci and is_container.
  • Implement _is_ci() and _is_container() heuristics (env vars, marker files, /proc/1/cgroup).
  • Add unit tests for the new helpers and system info schema.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
src/lightly_train/_events/tracker.py Adds CI/container detection and includes the flags in cached system info.
tests/_events/test_tracker.py Adds assertions for the new system info keys and unit tests for CI/container detection.

Comment thread tests/_events/test_tracker.py
Comment thread tests/_events/test_tracker.py
Comment thread tests/_events/test_tracker.py
Comment thread tests/_events/test_tracker.py
Comment thread src/lightly_train/_events/tracker.py Outdated
@guarin
Copy link
Copy Markdown
Contributor Author

guarin commented Mar 23, 2026

/review

1 similar comment
@guarin
Copy link
Copy Markdown
Contributor Author

guarin commented Mar 23, 2026

/review

Copy link
Copy Markdown
Contributor

@yutong-xiang-97 yutong-xiang-97 left a comment

Choose a reason for hiding this comment

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

Left some comments by AI

Comment thread tests/_events/test_tracker.py Outdated
Comment thread tests/_events/test_tracker.py
Comment thread tests/_events/test_tracker.py Outdated
Comment thread tests/_events/test_tracker.py Outdated
Comment thread src/lightly_train/_events/tracker.py Outdated
- Use /proc/self/cgroup instead of /proc/1/cgroup for accurate container
  detection when host PID namespace is shared
- Remove redundant CI env var cleanup in test__is_ci__false (clear=True
  already handles it)
- Add test for _is_ci with empty string value
- Add tests for _is_container exception paths (FileNotFoundError,
  PermissionError)
- Use clear=True in Singularity/Apptainer tests for env isolation
@guarin guarin enabled auto-merge (squash) March 23, 2026 15:56
@guarin guarin merged commit 570d6a6 into main Mar 23, 2026
16 checks passed
@guarin guarin deleted the trn-1839-log-ci-container branch March 23, 2026 16:08
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.

3 participants