fix(env): added missing CLICKHOUSE_USER environment variable - #270
fix(env): added missing CLICKHOUSE_USER environment variable#270nktnet1 wants to merge 8 commits into
Conversation
|
@nktnet1 is attempting to deploy a commit to the goldflag's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
I can't host Rybbit until this gets merged 😔 |
WalkthroughThe diff adds the Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / Orchestration
participant Backend as Backend service
participant ClickHouse as ClickHouse DB
Dev->>Backend: Start (docker-compose with CLICKHOUSE_USER)
Backend->>ClickHouse: Connect(url, database, username=CLICKHOUSE_USER, password)
ClickHouse-->>Backend: Auth response / connection established
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
mockdata/index.js (1)
6-11: Provide a sane fallback forusernamewhen the env var is absent.
Running the generator locally (outside Docker Compose) will setusername: undefined, which the ClickHouse JS client forwards verbatim and can trigger auth errors. A tiny default keeps the script zero-config.- username: process.env.CLICKHOUSE_USER, + // Default ClickHouse user is "default" when env not provided + username: process.env.CLICKHOUSE_USER || "default",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docker-compose.cloud.yml(1 hunks)docker-compose.yml(1 hunks)docs/src/content/self-hosting-advanced.mdx(1 hunks)docs/src/content/self-hosting-guides/nginx-proxy-manager.mdx(1 hunks)docs/src/content/self-hosting-manual.mdx(1 hunks)mockdata/index.js(1 hunks)server/src/db/clickhouse/clickhouse.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{client,server}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
{client,server}/**/*.{ts,tsx}: Use TypeScript with strict typing throughout both client and server
Use try/catch blocks with specific error types for error handling
Use camelCase for variables and functions, PascalCase for components and types
Group imports by external, then internal, and sort alphabetically within groups
Files:
server/src/db/clickhouse/clickhouse.ts
🧠 Learnings (7)
docs/src/content/self-hosting-manual.mdx (1)
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:10:29.485Z
Learning: Backend: Use Fastify, Drizzle ORM (Postgres), and ClickHouse
docker-compose.yml (1)
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:10:29.485Z
Learning: Backend: Use Fastify, Drizzle ORM (Postgres), and ClickHouse
docker-compose.cloud.yml (1)
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:10:29.485Z
Learning: Backend: Use Fastify, Drizzle ORM (Postgres), and ClickHouse
docs/src/content/self-hosting-advanced.mdx (1)
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:10:29.485Z
Learning: Backend: Use Fastify, Drizzle ORM (Postgres), and ClickHouse
server/src/db/clickhouse/clickhouse.ts (1)
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:10:29.485Z
Learning: Backend: Use Fastify, Drizzle ORM (Postgres), and ClickHouse
docs/src/content/self-hosting-guides/nginx-proxy-manager.mdx (1)
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:10:29.485Z
Learning: Backend: Use Fastify, Drizzle ORM (Postgres), and ClickHouse
mockdata/index.js (1)
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:10:29.485Z
Learning: Backend: Use Fastify, Drizzle ORM (Postgres), and ClickHouse
🔇 Additional comments (5)
docs/src/content/self-hosting-manual.mdx (1)
44-46: Environment snippet now documentsCLICKHOUSE_USER– nice catch.
Keeping docs in sync with new runtime variables prevents a lot of “works-on-my-machine” friction for self-hosters.docs/src/content/self-hosting-advanced.mdx (1)
64-67: Consistent advanced-guide update looks good.
The variable is optional and defaults match the container images – no further action needed.docker-compose.cloud.yml (1)
70-75: Backend now receivesCLICKHOUSE_USER– looks correct.
Value propagates from the same.envplaceholder used by the ClickHouse service, so auth will stay in sync.docker-compose.yml (1)
78-82: Compose file kept in parity with cloud variant – LGTM.
Nothing else to flag.docs/src/content/self-hosting-guides/nginx-proxy-manager.mdx (1)
74-74: LGTM! Properly documents the new environment variable.The addition of
CLICKHOUSE_USERenvironment variable with a default value follows the established pattern and aligns with the ClickHouse service configuration.
|
I was about to create the exact same PR, would be nice if it gets some traction from maintainers and get merged for the next release since it's such a simple and innocuous change. |
|
cc @goldflag |
|
Conflict has been resolved. |
|
@goldflag any change you can get a pass at this, please? |
|
What kind of incompetent bullsh*t is this? It's a minor, trivial issue and you guys haven't merged it for a year. Version 2.7.0 is still running with this incredibly dumb and ridiculous bug. |
|
Have resolved conflicts. Also, similar PR in #965. |
Since
CLICKHOUSE_USERcan be set as an environment variable, I assume the backend will also need knowledge of it, otherwise it will be out of sync with the clickhouse container for the first deploy if it's set to something other thandefault.Summary by CodeRabbit
New Features
Documentation
Chores