docs(clp-package): Document default container configurations in the package Docker image. #1530

junhaoliao · 2025-10-30T19:13:40Z

Description

Adds a configuration reference table to the clp-package Docker image documentation, documenting the default user, paths, and environment variables. This helps developers understand the container's runtime environment when debugging or extending the image.

Checklist

The PR satisfies the contribution guidelines.
This is a breaking change and that has been indicated in the PR title, OR this isn't a
breaking change.
Necessary docs have been updated, OR no docs need to be updated.

Validation performed

npx markdownlint-cli2 --config tools/yscope-dev-utils/exports/lint-configs/.markdownlint-cli2.yaml --fix docs/src/dev-docs/tooling-containers.md
markdownlint-cli2 v0.18.1 (markdownlint v0.38.0)
Finding: docs/src/dev-docs/tooling-containers.md
Linting: 1 file(s)
Summary: 0 error(s)

task docs:serve

Verified markdown formatting renders correctly
Confirmed table accurately reflects the Dockerfile configuration

Summary by CodeRabbit

Documentation
- Added a new "Default Configurations" subsection to the tooling containers documentation.
- Documents default environment variables (PATH, CLP_HOME, LD_LIBRARY_PATH, PYTHONPATH, SHELL, USER) and explains their roles for debugging and build environments.
- Purely informational update — no functional or API changes.

…ackage Docker image.

coderabbitai · 2025-10-30T19:13:52Z

Walkthrough

Added a "Default Configurations" subsection to the clp-package tooling-containers documentation, introducing a table that documents environment variable defaults (PATH, CLP_HOME, LD_LIBRARY_PATH, PYTHONPATH, SHELL, USER) and their purposes for debugging container builds.

Changes

Cohort / File(s)	Summary
Documentation `docs/src/dev-docs/tooling-containers.md`	Added "Default Configurations" subsection with a table documenting environment variable defaults (PATH, CLP_HOME, LD_LIBRARY_PATH, PYTHONPATH, SHELL, USER) and their purposes for container build debugging.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Pre-merge checks and finishing touches

❌ 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%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "docs(clp-package): Document default container configurations in the package Docker image." is fully related to the main change in the changeset. The raw summary confirms the PR adds a "Default Configurations" subsection with a table documenting environment-related defaults (PATH, CLP_HOME, LD_LIBRARY_PATH, PYTHONPATH, SHELL, USER) to the clp-package documentation in docs/src/dev-docs/tooling-containers.md. The title is specific, concise, and clearly communicates the primary purpose of the change—documenting default container configurations. The formatting follows the conventional commit style (docs(scope): description), making it professional and easy to scan in commit history.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2665244 and 7328954.

📒 Files selected for processing (1)

docs/src/dev-docs/tooling-containers.md (1 hunks)

🧰 Additional context used

🧠 Learnings (7)

📓 Common learnings

Learnt from: junhaoliao
Repo: y-scope/clp PR: 1414
File: tools/docker-images/clp-package/Dockerfile:20-24
Timestamp: 2025-10-13T03:32:19.293Z
Learning: In the clp repository's Dockerfiles (e.g., tools/docker-images/clp-package/Dockerfile), ENV directives should be split into separate lines for readability rather than consolidated to reduce layer count. This is especially true for PATH modifications, as agreed upon in PR #1166. Later ENV settings may depend on earlier ones (e.g., referencing CLP_HOME).

Learnt from: junhaoliao
Repo: y-scope/clp PR: 1178
File: components/clp-py-utils/clp_py_utils/clp_config.py:47-47
Timestamp: 2025-10-07T07:54:32.427Z
Learning: In components/clp-py-utils/clp_py_utils/clp_config.py, the CONTAINER_AWS_CONFIG_DIRECTORY constant is intentionally set to pathlib.Path("/") / ".aws" (i.e., `/.aws`) rather than a user-specific home directory. This hardcoded path is part of the container orchestration design.

Learnt from: junhaoliao
Repo: y-scope/clp PR: 1413
File: tools/docker-images/clp-package/Dockerfile:22-24
Timestamp: 2025-10-20T21:05:30.417Z
Learning: In the clp repository's Dockerfiles, ENV directives should be consolidated into multi-line ENV statements when possible to reduce image layers. ENV statements should only be split into separate commands when consolidation is not possible due to dependencies (e.g., when later variables must reference earlier ones that need to be set first, or when PATH must be modified sequentially).

📚 Learning: 2025-10-13T03:32:19.293Z

Learnt from: junhaoliao
Repo: y-scope/clp PR: 1414
File: tools/docker-images/clp-package/Dockerfile:20-24
Timestamp: 2025-10-13T03:32:19.293Z
Learning: In the clp repository's Dockerfiles (e.g., tools/docker-images/clp-package/Dockerfile), ENV directives should be split into separate lines for readability rather than consolidated to reduce layer count. This is especially true for PATH modifications, as agreed upon in PR #1166. Later ENV settings may depend on earlier ones (e.g., referencing CLP_HOME).

Applied to files:

docs/src/dev-docs/tooling-containers.md

📚 Learning: 2025-09-25T05:13:13.298Z

Learnt from: junhaoliao
Repo: y-scope/clp PR: 1178
File: components/clp-package-utils/clp_package_utils/controller.py:217-223
Timestamp: 2025-09-25T05:13:13.298Z
Learning: The compression scheduler service in CLP runs with CLP_UID_GID (current user's UID:GID) rather than CLP_SERVICE_CONTAINER_UID_GID (999:999), unlike infrastructure services such as database, queue, redis, and results cache which run with the service container UID:GID.

Applied to files:

docs/src/dev-docs/tooling-containers.md

📚 Learning: 2025-10-20T21:05:30.417Z

Learnt from: junhaoliao
Repo: y-scope/clp PR: 1413
File: tools/docker-images/clp-package/Dockerfile:22-24
Timestamp: 2025-10-20T21:05:30.417Z
Learning: In the clp repository's Dockerfiles, ENV directives should be consolidated into multi-line ENV statements when possible to reduce image layers. ENV statements should only be split into separate commands when consolidation is not possible due to dependencies (e.g., when later variables must reference earlier ones that need to be set first, or when PATH must be modified sequentially).

Applied to files:

docs/src/dev-docs/tooling-containers.md

📚 Learning: 2025-10-07T07:54:32.427Z

Learnt from: junhaoliao
Repo: y-scope/clp PR: 1178
File: components/clp-py-utils/clp_py_utils/clp_config.py:47-47
Timestamp: 2025-10-07T07:54:32.427Z
Learning: In components/clp-py-utils/clp_py_utils/clp_config.py, the CONTAINER_AWS_CONFIG_DIRECTORY constant is intentionally set to pathlib.Path("/") / ".aws" (i.e., `/.aws`) rather than a user-specific home directory. This hardcoded path is part of the container orchestration design.

Applied to files:

docs/src/dev-docs/tooling-containers.md

📚 Learning: 2025-06-18T20:48:48.990Z

Learnt from: quinntaylormitchell
Repo: y-scope/clp PR: 968
File: docs/src/user-guide/quick-start/overview.md:53-54
Timestamp: 2025-06-18T20:48:48.990Z
Learning: CLP is designed to run on Linux systems where Python is typically pre-installed, so Python installation links are generally not needed in CLP documentation.

Applied to files:

docs/src/dev-docs/tooling-containers.md

📚 Learning: 2025-08-25T16:27:50.549Z

Learnt from: davemarco
Repo: y-scope/clp PR: 1198
File: components/webui/server/src/plugins/app/Presto.ts:38-43
Timestamp: 2025-08-25T16:27:50.549Z
Learning: In the CLP webui Presto configuration, host and port are set via package settings (configurable), while user, catalog, and schema are set via environment variables (environment-specific). This mixed approach is intentional - settings are typically set by package and some values don't need to be package-configurable.

Applied to files:

docs/src/dev-docs/tooling-containers.md

🔇 Additional comments (1)

docs/src/dev-docs/tooling-containers.md (1)

142-155: Solid documentation addition with clear presentation of container defaults.

The new subsection effectively documents the default configurations, and the table format makes the information scannable and maintainable. The introductory context appropriately clarifies these are for understanding the container runtime environment rather than direct production use.

To ensure accuracy and maintainability, please verify that all values in the table (especially the UID 1000, paths like /opt/clp, and shell /bin/bash) match the current Dockerfile configuration in tools/docker-images/clp-package/Dockerfile. Given that documentation can drift from implementation over time, consider adding a comment or note in the markdown indicating which version of the Dockerfile these defaults reflect (e.g., "as of commit X" or a reference to a specific Dockerfile section).

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

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

sitaowang1998 · 2025-10-30T19:58:55Z

docs/src/dev-docs/tooling-containers.md

+While not intended for direct end-user deployment, the container includes these configurations for
+debugging purposes:


I don't think these configurations are really for debugging.

Technically we don't have to create users in the image and any env var overrides can be specified only in the docker compose yamls. That said, practically we can clean up the duplicated specifications from the docker compose files now they are specified in the image

If we don't clean up, until we make the Package docker image standalone runnable, the non root user and the env vars are to help debugging the Package runtime environment

docs/src/dev-docs/tooling-containers.md

Co-authored-by: Bingran Hu <[email protected]>

kirkrodrigues · 2025-11-05T03:54:36Z

docs/src/dev-docs/tooling-containers.md

+While not intended for direct end-user deployment, the container includes these configurations for
+debugging purposes:
+
+| Setting         | Related Env Var   | Value                                                  | Purpose                                          |


I think some of these deserve more explanation. I'll come back to this PR after the release.

docs(clp-package): Document default container configurations in the p…

2665244

…ackage Docker image.

junhaoliao requested a review from a team as a code owner October 30, 2025 19:13

junhaoliao requested review from Bill-hbrhbr, kirkrodrigues and sitaowang1998 October 30, 2025 19:14

sitaowang1998 reviewed Oct 30, 2025

View reviewed changes

Bill-hbrhbr reviewed Oct 31, 2025

View reviewed changes

docs/src/dev-docs/tooling-containers.md Outdated Show resolved Hide resolved

junhaoliao and others added 2 commits October 31, 2025 17:15

grammar - Apply suggestions from code review

7328954

Co-authored-by: Bingran Hu <[email protected]>

Merge branch 'main' into docker-image-docs

3ed81bb

junhaoliao requested review from Bill-hbrhbr and sitaowang1998 November 4, 2025 18:19

sitaowang1998 approved these changes Nov 4, 2025

View reviewed changes

Bill-hbrhbr approved these changes Nov 5, 2025

View reviewed changes

kirkrodrigues reviewed Nov 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

docs(clp-package): Document default container configurations in the package Docker image. #1530

docs(clp-package): Document default container configurations in the package Docker image. #1530

Uh oh!

junhaoliao commented Oct 30, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Oct 30, 2025 •

edited

Loading

Uh oh!

sitaowang1998 Oct 30, 2025

Uh oh!

junhaoliao Oct 31, 2025

Uh oh!

Uh oh!

kirkrodrigues Nov 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		While not intended for direct end-user deployment, the container includes these configurations for
		debugging purposes:

docs(clp-package): Document default container configurations in the package Docker image. #1530

Are you sure you want to change the base?

docs(clp-package): Document default container configurations in the package Docker image. #1530

Uh oh!

Conversation

junhaoliao commented Oct 30, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Validation performed

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

sitaowang1998 Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

junhaoliao Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kirkrodrigues Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

junhaoliao commented Oct 30, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 30, 2025 •

edited

Loading