Skip to content

feat(clickhouse): adding user creation support#2746

Open
szucsitg wants to merge 9 commits into
cachix:mainfrom
szucsitg:clickhouse-fixes
Open

feat(clickhouse): adding user creation support#2746
szucsitg wants to merge 9 commits into
cachix:mainfrom
szucsitg:clickhouse-fixes

Conversation

@szucsitg
Copy link
Copy Markdown

Current Clickhouse implementation doesn't allow to create a user or modify any behavior.
Additionally I run into issues when try to run migration scripts due to lacking keeper integration and configuration for it.
Also macro support wasn't enabled.

Comment thread src/modules/services/clickhouse.nix Outdated
Comment on lines +73 to +74
mysql_port: 0
postgresql_port: 0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What are these for?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i wanted to explicitly disabled it in my case, but looking at deeper into the config (clickhouse docs are sometimes hard to follow), it's not necessary

Comment thread src/modules/services/clickhouse.nix Outdated
Comment thread src/modules/services/clickhouse.nix
Comment thread src/modules/services/clickhouse.nix
@szucsitg
Copy link
Copy Markdown
Author

Thanks for your comments, I’ll get back to you with some changes based on your feedback. I also found a few minor issues in the last few days while I was testing my changes.

@szucsitg szucsitg requested a review from sandydoo April 22, 2026 11:38
Copy link
Copy Markdown
Member

@sandydoo sandydoo left a comment

Choose a reason for hiding this comment

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

Looking good! My only request is to rename the options.

For example, instead of keeperEnable, we prefer keeper.enable. Same for the rest.
I'll make sure to document this somewhere for the future.

@szucsitg
Copy link
Copy Markdown
Author

@sandydoo i made the changes

Comment on lines +47 to +57
keeperPort = lib.mkOption {
type = types.port;
description = "Which port to run clickhouse keeper service on.";
default = 9181;
};

raftPort = lib.mkOption {
type = types.port;
description = "Which http port to use clickhouse keeper for raft consensus.";
default = 9234;
};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

keeperPort -> keeper.port.

raftPort -> keeper.raft.port? Does nesting under keeper make sense? Not familiar with this thing.

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