Skip to content

fix(env): added missing CLICKHOUSE_USER environment variable - #270

Open
nktnet1 wants to merge 8 commits into
rybbit-io:masterfrom
nktnet1:clickhouse-user
Open

fix(env): added missing CLICKHOUSE_USER environment variable#270
nktnet1 wants to merge 8 commits into
rybbit-io:masterfrom
nktnet1:clickhouse-user

Conversation

@nktnet1

@nktnet1 nktnet1 commented May 16, 2025

Copy link
Copy Markdown
Contributor

Since CLICKHOUSE_USER can 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 than default.

Summary by CodeRabbit

  • New Features

    • Added support for specifying the ClickHouse user via a new environment variable for backend and compose deployments.
  • Documentation

    • Updated self-hosting and setup guides to include the optional ClickHouse user environment variable in examples.
  • Chores

    • Backend client now respects the configured ClickHouse username from environment variables for authentication.

@vercel

vercel Bot commented May 16, 2025

Copy link
Copy Markdown

@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.

@FraglyG

FraglyG commented Jun 14, 2025

Copy link
Copy Markdown

I can't host Rybbit until this gets merged 😔

@coderabbitai

coderabbitai Bot commented Jul 24, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The diff adds the CLICKHOUSE_USER environment variable (default default) to Docker Compose services and docs, and updates ClickHouse client initialization in server and mockdata to include username from that variable for authentication.

Changes

Cohort / File(s) Change Summary
Compose files
docker-compose.yml, docker-compose.cloud.yml
Added CLICKHOUSE_USER=default to the backend service environment.
Documentation
docs/src/content/self-hosting-advanced.mdx, docs/src/content/self-hosting-manual.mdx, docs/src/content/self-hosting-guides/nginx-proxy-manager.mdx
Added CLICKHOUSE_USER to example .env snippets and Docker Compose examples (commented or explicit).
Runtime / mocks
mockdata/index.js
Pass username: process.env.CLICKHOUSE_USER into ClickHouse client configuration.
Server DB client
server/src/db/clickhouse/clickhouse.ts
Added username property to ClickHouse client initialization using process.env.CLICKHOUSE_USER.

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
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A rabbit taps keys in the night,
Adding a name so auth is right.
Env vars aligned,
Connections signed —
ClickHouse greets with a hop and a bite. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and accurately summarizes the primary change—adding the missing CLICKHOUSE_USER environment variable—and aligns with the changes to docker-compose files, documentation, and ClickHouse client configuration in the diff. It is concise, specific, and uses a conventional-commit style prefix so a reviewer scanning history will understand the intent. There are no extraneous details, emojis, or vague phrasing that would obscure the main change.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f503d1 and f1ee75b.

📒 Files selected for processing (2)
  • docker-compose.cloud.yml (1 hunks)
  • docker-compose.yml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • docker-compose.yml
  • docker-compose.cloud.yml
⏰ 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)
  • GitHub Check: Build Client Images (ubuntu-latest, linux/amd64)
  • GitHub Check: Build Client Images (ubuntu-24.04-arm, linux/arm64)

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.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

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.

❤️ Share

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

@nktnet1 nktnet1 changed the title fix: added missing CLICKHOUSE_USER env fix(env): added missing CLICKHOUSE_USER environment variable Jul 24, 2025

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
mockdata/index.js (1)

6-11: Provide a sane fallback for username when the env var is absent.
Running the generator locally (outside Docker Compose) will set username: 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

📥 Commits

Reviewing files that changed from the base of the PR and between 895ca2d and 6f503d1.

📒 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 documents CLICKHOUSE_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 receives CLICKHOUSE_USER – looks correct.
Value propagates from the same .env placeholder 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_USER environment variable with a default value follows the established pattern and aligns with the ClickHouse service configuration.

Comment thread server/src/db/clickhouse/clickhouse.ts
@Hiruma31

Copy link
Copy Markdown

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.
The automated code review suggests something that is not even done on the other params.

@nktnet1

nktnet1 commented Sep 13, 2025

Copy link
Copy Markdown
Contributor Author

cc @goldflag

@codingCoffee

Copy link
Copy Markdown

@nktnet1 maybe resolve the merge conflicts once
@goldflag can you maybe look at the PR once, seems like a trivial fix; ran into the same issue while hosting rybbit

@nktnet1

nktnet1 commented Dec 23, 2025

Copy link
Copy Markdown
Contributor Author

Conflict has been resolved.

@jfmlima

jfmlima commented Mar 5, 2026

Copy link
Copy Markdown

@goldflag any change you can get a pass at this, please?

@jfmlima

jfmlima commented Mar 5, 2026

Copy link
Copy Markdown

@nktnet1 @FraglyG setting CLICKHOUSE_HOST with something like http[s]://[username:password@]hostname:port[/database] works, just fyi.

@Manta7749

Copy link
Copy Markdown

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.

@nktnet1

nktnet1 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Have resolved conflicts. Also, similar PR in #965.

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.

6 participants