Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add role support via query parameters #269

Open
slvrtrn opened this issue May 17, 2024 · 2 comments · May be fixed by #328
Open

Add role support via query parameters #269

slvrtrn opened this issue May 17, 2024 · 2 comments · May be fixed by #328
Assignees
Labels
enhancement New feature or request

Comments

@slvrtrn
Copy link
Contributor

slvrtrn commented May 17, 2024

@slvrtrn slvrtrn added the enhancement New feature or request label May 17, 2024
@slvrtrn slvrtrn self-assigned this May 17, 2024
@pulpdrew
Copy link

@slvrtrn Are you actively working on this, or would it be alright if I attempted it?

@slvrtrn
Copy link
Contributor Author

slvrtrn commented Sep 20, 2024

@pulpdrew, sure, please feel free to do so.
I think the impl is more or less as follows:
We should have

role: string | Array<string>

added to BaseClickHouseClientConfigOptions (client-common/src/config.ts)

  • also add it to QueryParams, CommandParams, ExecParams, InsertParams (client-common/src/client.ts).
  • Similarly, it should be added to these interfaces' counterparts in the client-common/src/connection.ts.
  • Add roles to the toSearchParams fn in common/utils/url.ts
  • Finally, the Node.js client - pass roles to the toSearchParams fn in the client-node/src/connection/node_base_connection.ts; Web client - same in the client-web/src/connection/web_connection.
  • Probably not worth adding it to the URL configuration currently
  • Tests: this is the annoying part. Probably a separate file in the common pkg tests for all the possibilities (global, overrides from the query, exec, insert, command methods); maybe restrict it to the single node only for simplicity, cause there is quite a lot of bootstrap, and it's getting more complicated with a cluster.

In case of any questions, please feel free to contact me in the community Slack.

NB: in the case of just a single role now, the current workaround could be

const client = createClient({
  clickhouse_settings: {
    role: 'my_role',
  }
})

should work similarly with the other methods (query, insert, command, exec). But this will not allow to provide multiple roles.

@pulpdrew pulpdrew linked a pull request Sep 25, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants