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

No Kafka ElasticSearch enable magic #60

Merged
merged 4 commits into from
Mar 8, 2024

Conversation

minijackson
Copy link
Collaborator

This removes the logic where "if the Kafka server configured in phoebus-alarm-server/phoebus-alarm-logger is localhost, then automagically enable Kafka", and same for ElasticSearch.

Now the user always has to configure Kafka and ElasticSearch themselves, and we document how to do it in the Phoebus alarm guide.

Fixes #54.

@minijackson minijackson self-assigned this Feb 13, 2024
@minijackson minijackson changed the title No Kafka Elasticsearch magic No Kafka ElasticSearch enable magic Feb 13, 2024
Copy link
Collaborator

@stephane-cea stephane-cea left a comment

Choose a reason for hiding this comment

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

This looks good to me, thx!

now the user must configure Kafka and ElasticSearch themselves
test would fail because the the list returned by phoebus-alarm-logger
wouldn't be sorted by time, which we assumed was the case.
@minijackson minijackson force-pushed the no-kafka-elasticsearch-magic branch from 5870e02 to 8a99eee Compare February 13, 2024 13:21
@lcaouen
Copy link
Collaborator

lcaouen commented Feb 29, 2024

Tests are ok except a time difference between the alarm time and the message time.

Copy link
Collaborator

@lcaouen lcaouen left a comment

Choose a reason for hiding this comment

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

Changes are ok

@minijackson minijackson merged commit e968026 into master Mar 8, 2024
4 checks passed
@minijackson minijackson deleted the no-kafka-elasticsearch-magic branch March 8, 2024 12:28
minijackson added a commit that referenced this pull request Apr 29, 2024
minijackson added a commit that referenced this pull request Apr 29, 2024
see #75, #54, follow-up of #60

Specify in documentation that enabling ElasticSearch needs to be done
only once.
minijackson added a commit to minijackson/EPNix that referenced this pull request Jul 30, 2024
see epics-extensions#75, epics-extensions#54, follow-up of epics-extensions#60

Specify in documentation that enabling ElasticSearch needs to be done
only once.
minijackson added a commit to minijackson/EPNix that referenced this pull request Jul 30, 2024
see epics-extensions#75, epics-extensions#54, follow-up of epics-extensions#60

Specify in documentation that enabling ElasticSearch needs to be done
only once.
minijackson added a commit to minijackson/EPNix that referenced this pull request Aug 1, 2024
see epics-extensions#75, epics-extensions#54, follow-up of epics-extensions#60

Specify in documentation that enabling ElasticSearch needs to be done
only once.
minijackson added a commit to minijackson/EPNix that referenced this pull request Aug 2, 2024
see epics-extensions#75, epics-extensions#54, follow-up of epics-extensions#60

Specify in documentation that enabling ElasticSearch needs to be done
only once.
github-actions bot pushed a commit that referenced this pull request Aug 2, 2024
github-actions bot pushed a commit that referenced this pull request Aug 2, 2024
see #75, #54, follow-up of #60

Specify in documentation that enabling ElasticSearch needs to be done
only once.

(cherry picked from commit 366ba04)
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.

EPNix shouldn't rely on localhost: to figure out whether to enable kafka or not
3 participants