Skip to content
New issue

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

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

Already on GitHub? # to your account

feat: added additional headers to clickhouse requests #224

Merged
merged 2 commits into from
Jan 19, 2024
Merged

feat: added additional headers to clickhouse requests #224

merged 2 commits into from
Jan 19, 2024

Conversation

teawithfruit
Copy link
Contributor

Summary

This PR enables clickhouse-js to use additional headers in http(s) requests. On my side it was necessary to be able to use a reverse proxy with basic auth in front. For example, something like this is possible from now on.

createClient({
  host: 'https://log.example.com',
  username: 'basic_auth_user',
  password: 'basic_auth_password',
  additionalHeaders: {
    'X-ClickHouse-User': 'clickhouse_user',
    'X-ClickHouse-Key': 'clickhouse_password',
  },
});

Checklist

Delete items not relevant to your PR:

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG
  • For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

@CLAassistant
Copy link

CLAassistant commented Jan 17, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@slvrtrn slvrtrn left a comment

Choose a reason for hiding this comment

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

Can you please

  • add these options to the web version as well
  • add tests - for the web version, the suitable place likely will be in packages/client-web/__tests__/integration/web_connection.test.ts (fetch stubs are already there, should be easy to assert on headers); for Node.js, a similar one can be created as packages/client-node/__tests__/integration/node_connection.test.ts as we want to test the entire interaction (client options -> connection -> request). There are also examples of http.request stubs here.

@teawithfruit teawithfruit requested a review from slvrtrn January 18, 2024 17:40
@slvrtrn slvrtrn merged commit 9b358ea into ClickHouse:main Jan 19, 2024
@slvrtrn
Copy link
Contributor

slvrtrn commented Jan 19, 2024

@teawithfruit, thanks for the contribution. I took the liberty to do some fixups in the tests, plus I noticed that Record<string, number | string | string[]> was a bit too permissive (fetch API is a bit more strict on that), so I reduced it to Record<string, string> as it matches both APIs.

Released as 0.2.9.

@teawithfruit
Copy link
Contributor Author

Thanks for reacting such fast. 🙂

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants