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

Upgrade stomp to 8.0.1 #832

Merged
merged 2 commits into from
May 2, 2022
Merged

Upgrade stomp to 8.0.1 #832

merged 2 commits into from
May 2, 2022

Conversation

jertel
Copy link
Owner

@jertel jertel commented May 2, 2022

Description

Upgrade Stomp to 8.0.1

Checklist

  • I have reviewed the contributing guidelines.
  • [NA] I have included unit tests for my changes or additions.
  • I have successfully run make test-docker with my changes.
  • [NA] I have manually tested all relevant modes of the change in this PR.
  • [NA] I have updated the documentation.
  • I have updated the changelog.

Questions or Comments

@jertel jertel requested a review from nsano-rururu May 2, 2022 13:04
@nsano-rururu
Copy link
Collaborator

It seems that the use_ssl parameter has been removed in stomp 8.0. Therefore, the following error is displayed.
As a response, I think it is necessary to either implement a new setting or temporarily delete the ssl setting.

https://github.com/jasonrbriggs/stomp.py/blob/dev/CHANGELOG.md#version-800---feb-2022

Remove deprecated constructor params (use_ssl, and other ssl params)

error message

ERROR:elastalert:Error while running alert stomp: Error posting to Stomp: __init__() got an unexpected keyword argument 'use_ssl'

a.yaml

alert:
  - "stomp"
alert_text_type: alert_text_jinja
#alert_text: "Test {}  123 bb☃"
alert_text: |
  Alert triggered! {{message}}
alert_subject: "Test {} 123 aa☃"
alert_subject_args:
  - "message"
  - "@log_name"
alert_text_args:
  - "message"
filter:
  - query:
      query_string:
        query: "message:Quit"
index: "mariadblog-*"
is_enabled: true
name: "a"
num_events: 2
realert:
  minutes: 5
terms_size: 50
timeframe:
  minutes: 5
timestamp_field: "@timestamp"
timestamp_type: "iso"
type: "frequency"
use_strftime_index: false
stomp_hostname: "localhost"
stomp_hostport: "61613"
stomp_login: "guest"
stomp_password: "guest"
stomp_destination: "/queue/ALERT"

@nsano-rururu
Copy link
Collaborator

Since it is a function that does not receive any inquiries from users, I think it is okay to delete the ssl connection once.

  • Removed description of stomp_ssl from ruletypes.html # stomp
  • Remove stomp_ssl from schema.yaml
  • Remove stomp_ssl from stomp.py

delete

self.stomp_ssl = self.rule.get('stomp_ssl', False)

modify

conn = stomp.Connection([(self.stomp_hostname, self.stomp_hostport)], use_ssl=self.stomp_ssl)

to

conn = stomp.Connection([(self.stomp_hostname, self.stomp_hostport)])

@jertel
Copy link
Owner Author

jertel commented May 2, 2022

I think that answers the question of whether this alerter is being used. It's clearly not, since this must have been broken since the Stomp.py package was updated to 8.0.0 in ElastAlert2 2.4.0, which was released almost 2 months ago. And no one has raised an issue that the stomp alerter was broken.

@jertel jertel merged commit dfa7866 into master May 2, 2022
@jertel jertel deleted the stompup branch May 2, 2022 21:39
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants