Skip to content

Conversation

macobo
Copy link
Contributor

@macobo macobo commented Jan 28, 2022

This is needed when using an external clickhouse which doesn't have the
same access to kafka as in-cluster traffic does.

Note that long-term we might need to also provide better auth mechanisms
here as well.

Related: PostHog/charts-clickhouse#276

@macobo macobo requested a review from guidoiaquinti January 28, 2022 09:27
@macobo macobo force-pushed the clickhouse-kafka-hosts branch from d9e03b6 to e7575d7 Compare January 28, 2022 09:36
…nv var

This is needed when using an external clickhouse which doesn't have the
same access to kafka as in-cluster traffic does.

Note that long-term we might need to also provide better auth mechanisms
here as well.
@macobo macobo force-pushed the clickhouse-kafka-hosts branch from e7575d7 to 6ea6590 Compare January 28, 2022 09:45
KAFKA_HOSTS = _parse_kafka_hosts(KAFKA_URL)

# Kafka broker host(s) that is used by clickhouse for ingesting messages. Useful if clickhouse is hosted outside the cluster.
KAFKA_HOSTS_CLICKHOUSE = _parse_kafka_hosts(os.getenv("KAFKA_URL_CLICKHOUSE", KAFKA_URL))
Copy link
Contributor

Choose a reason for hiding this comment

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

What about KAFKA_URL_EXTERNAL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not used anywhere hence thought best not to expose it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From discussion: KAFKA_HOSTS_FOR_CLICKHOUSE and similar name for the env var :)

Copy link
Contributor

@guidoiaquinti guidoiaquinti left a comment

Choose a reason for hiding this comment

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

CI is failing but overall LGTM

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