Skip to content

chore: remove ch provision users#4047

Closed
MasterPtato wants to merge 1 commit intographite-base/4047from
01-26-chore_remove_ch_provision_users
Closed

chore: remove ch provision users#4047
MasterPtato wants to merge 1 commit intographite-base/4047from
01-26-chore_remove_ch_provision_users

Conversation

@MasterPtato
Copy link
Contributor

No description provided.

@railway-app
Copy link

railway-app bot commented Jan 27, 2026

🚅 Deployed to the rivet-pr-4047 environment in rivet-frontend

Service Status Web Updated (UTC)
frontend-cloud 🕗 Deploying (View Logs) Web Jan 27, 2026 at 3:19 am
mcp-hub ✅ Success (View Logs) Web Jan 27, 2026 at 3:18 am
frontend-inspector 🕒 Building (View Logs) Web Jan 27, 2026 at 3:18 am
website 🕒 Building (View Logs) Web Jan 27, 2026 at 3:17 am

Copy link
Contributor Author

MasterPtato commented Jan 27, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@MasterPtato MasterPtato marked this pull request as ready for review January 27, 2026 03:17
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 27, 2026

Merge activity

  • Jan 27, 3:18 AM UTC: MasterPtato added this pull request to the Graphite merge queue.
  • Jan 27, 3:18 AM UTC: CI is running for this pull request on a draft pull request (#4049) due to your merge queue CI optimization settings.
  • Jan 27, 3:19 AM UTC: Merged by the Graphite merge queue via draft PR: #4049.

graphite-app bot pushed a commit that referenced this pull request Jan 27, 2026
@claude
Copy link

claude bot commented Jan 27, 2026

PR Review: Remove ClickHouse Provision Users

Summary

This PR removes the provision_users configuration and related types (ClickHouseUser, ClickHouseUserRole) from the ClickHouse configuration. The changes are clean and appear to be part of a deliberate simplification.

Code Quality ✅

  • Clean removal: All related types and fields removed consistently
  • No orphaned code: Removed HashMap import that's no longer needed
  • Follows conventions: Changes align with the codebase structure

Issues Found ⚠️

1. Generated Docker configs not updated (Important)

The PR modifies the template at engine/docker/template/src/services/edge/rivet-engine.ts, but the generated Docker config files still contain provision_users:

engine/docker/dev/rivet-engine/config.jsonc
engine/docker/dev-multinode/rivet-engine/*/config.jsonc
engine/docker/dev-multidc/datacenters/*/rivet-engine/config.jsonc
... and 14+ other generated configs

Action needed: According to CLAUDE.md, you should regenerate the Docker configs:

cd docker/template && pnpm start

2. Schema artifact may need regeneration

The file engine/artifacts/config-schema.json likely still references the removed types. This should be regenerated after the Rust changes.

3. Missing context in PR description

The PR body is empty. Consider adding:

  • Why provision_users is being removed
  • Whether this functionality moved elsewhere or is no longer needed
  • Any migration steps for existing deployments

Testing Recommendations 🧪

  1. Verify the config package compiles: cargo build -p config
  2. Regenerate Docker templates and verify they work
  3. Test that ClickHouse connections still work without the provision users configuration
  4. Ensure existing deployments that may have provision_users in their configs handle the field gracefully (should be ignored due to deny_unknown_fields removal or config migration)

Security & Performance ✅

  • No security concerns introduced
  • No performance impact
  • Simplification reduces configuration surface area (positive)

Overall Assessment

The code changes are correct, but the PR is incomplete because the Docker template regeneration step was skipped. This will cause inconsistency between the template source and generated configs.

Recommendation: Complete the Docker config regeneration before merging.

@graphite-app graphite-app bot changed the base branch from 01-23-fix_move_actor_metrics_to_separate_workflow to graphite-base/4047 January 27, 2026 03:19
@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 27, 2026

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@4047

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@4047

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@4047

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@4047

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@4047

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@4047

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@4047

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4047

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@4047

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@4047

commit: 8182268

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.

1 participant