8000 fix(metrics): add clickhouse schema initialization step and set worker permissions by andrzejfudro · Pull Request #681 · google/xmc · GitHub
[go: up one dir, main page]

Skip to content

fix(metrics): add clickhouse schema initialization step and set worker permissions - #681

Merged
andrzejfudro merged 2 commits into
mainfrom
fix/clickhouse-schema-docs-and-permissions
Aug 31, 2026
Merged

andrzejfudro merged 2 commits into
mainfrom
fix/clickhouse-schema-docs-and-permissions

Conversation

@andrzejfudro
@andrzejfudro andrzejfudro commented Aug 31, 2026 •
Copy link
Copy Markdown
Collaborator

Two small findings from recent test deployment.

  1. Missing documentation step for initializing clickhouse schema
  2. Chmod in worker dockerfile

…r permissions

- Add Step 6 to deploy/addons/metrics/clickhouse/README.md to document applying deploy/clickhouse/schema.sql after ClickHouse Helm installation.
- Update examples/clickhouse_metrics_job/Dockerfile to set world-readable permissions (chmod 755 /app /app/worker.py) so the container runs reliably under non-root user UID 1000 in GKE without Errno 13 permission denied.

TAG=agy
CONV=2181645e-ca2d-4226-90b5-e4a821928f9d
@gemini-code-assist gemini-code-assist Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request adds documentation for initializing the ClickHouse metrics schema and updates file permissions in the metrics job Dockerfile. The review feedback suggests improving the documentation by targeting the StatefulSet instead of a hardcoded pod name, and applying the principle of least privilege by setting the Python script permissions to 644 instead of 755, as it is executed directly via the python3 interpreter.

…missions to 644

- Target statefulset/my-clickstack-clickhouse-clickhouse-0-0 in deploy/addons/metrics/clickhouse/README.md Step 6.
- Update examples/clickhouse_metrics_job/Dockerfile to set chmod 644 on worker.py.

TAG=agy
CONV=2181645e-ca2d-4226-90b5-e4a821928f9d
@andrzejfudro
andrzejfudro requested a review from AntonTyb August 31, 2026 14:30
@andrzejfudro
andrzejfudro marked this pull request as ready for review August 31, 2026 14:30
@andrzejfudro
andrzejfudro added this pull request to the merge queue Aug 31, 2026
Merged via the queue into main with commit 31a7e23 Aug 31, 2026
12 checks passed
@andrzejfudro
andrzejfudro deleted the fix/clickhouse-schema-docs-and-permissions branch August 31, 2026 14:34
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.

2 participants

0