Skip to content

Conversation

@shroffk
Copy link
Member

@shroffk shroffk commented Oct 22, 2025

Checklist

  • Testing:

    • tested deploying on webpods and verified alarm servers default actions ( add/remove/modify configuration, generate alarms, ack/unack, enable/disable )
  • Documentation:

    • The feature is documented
    • The documentation is up to date
    • Release notes:
      • Added an entry if the change is breaking or significant
      • Added an entry when adding a new feature

@shroffk shroffk requested review from georgweiss and kasemir October 22, 2025 14:32
@shroffk
Copy link
Member Author

shroffk commented Oct 22, 2025

I have update the alarm server startup so that it will create and properly configure the kafka topics based on the config_name

It basically does that things the create_alarm_topics.sh does... with the added benefit that we don't need to remind anyone.

Note:
I have tested that it create new ones and fixes existing topics... but would prefer a bit more looking into

Copy link
Collaborator

@kasemir kasemir left a comment

Choose a reason for hiding this comment

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

Good idea!

@kasemir
Copy link
Collaborator

kasemir commented Oct 24, 2025

As for "The feature is documented", it's documented that you need to call create_alarm_topics.sh, which is no longer the case.
That script can be removed. The documentation should probably mention that we rely on a "compacting" topic for the alarm config and state, and how that's done automatically by running the alarm server for the first time on a new config.

"\trestart - Re-load alarm configuration and restart.\n" +
"\tshutdown - Shut alarm server down and exit.\n";

private static void ensureKafkaTopics(String server, String topic, String kafka_props_file) throws Exception {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The kafka_props_file argument is not used in this method.

Copy link
Collaborator

@georgweiss georgweiss left a comment

Choose a reason for hiding this comment

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

All seems to work as expected.

But I've added some comments.

@shroffk shroffk merged commit ded1e0b into master Oct 29, 2025
3 checks passed
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.

4 participants