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 basic topic config for energy-budget-plan namespace #164

Merged
merged 9 commits into from
Feb 29, 2024

Conversation

cdempsie
Copy link
Contributor

  • Creates 3 topics for energy-budget-plan: energy-budget-plan.eqdb-loader, energy-budget-plan.budget-plan and energy-budget-plan.customer-change
  • Configures an EQDB loader process as producer on energy-budget-plan.eqdb-loader
  • Configures a Budget Plan Fabricator process as the consumer of energy-budget-plan.eqdb-loader and producer on energy-budget-plan.customer-change
  • Configures Budge Plan Calculator as consumer and producer of energy-budget-plan.budget-plan
  • Cert config in wip PR https://github.com/utilitywarehouse/kubernetes-manifests/pull/85143

@cdempsie cdempsie requested a review from a team as a code owner February 29, 2024 13:57
Copy link

linear bot commented Feb 29, 2024

Comment on lines 59 to 64
module "budget_plan_fabricator" {
source = "../../../modules/tls-app"
consume_topics = { (kafka_topic.eqdb_loader.name) : "energy-budget-plan.eqdb-fabricator-v1" }
produce_topics = [kafka_topic.customer_change.name]
cert_common_name = "energy-budget-plan/eqdb-fabricator"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The way this is currently built it consumes its own messages. We can change that of course but maybe we add in consume on the customer_change too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, can do that while I am here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@utilitywarehouse/pubsub do consumer group names have to be unique or can I specify the same consumer group name on different topics?

Copy link
Contributor

@sbuliarca sbuliarca Feb 29, 2024

Choose a reason for hiding this comment

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

Hmmm, this is a good question. The way the tls-app is module is working right now, doing that would create the group_acl resource twice https://github.com/utilitywarehouse/kafka-cluster-config/blob/main/modules/tls-app/main.tf#L13-L21, so I think Terraform would be confused by that.

I'll look into it and see if we can adjust that, but for now I would suggest a different consumer group.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made the consumer groups unique for now.

Copy link
Contributor

@sbuliarca sbuliarca left a comment

Choose a reason for hiding this comment

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

LGTM

@cdempsie cdempsie merged commit ad4033c into main Feb 29, 2024
1 check passed
@cdempsie cdempsie deleted the sfe-417-basic-setup branch February 29, 2024 16:11
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.

3 participants