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

Fixing get_rule_file_hash to always return bytes #566

Merged
merged 2 commits into from
Nov 22, 2021

Conversation

JeffAshton
Copy link
Contributor

@JeffAshton JeffAshton commented Nov 22, 2021

Description

Fixing get_rule_file_hash for the case where an import is changed or removed, and the cached import path no longer exists. get_rule_file_hash will now always return bytes.

"Job "Internal: Handle Config Change (trigger: interval[0:02:30], next run at: 2021-11-19 21:05:16 UTC)" raised an exception
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/apscheduler/executors/base.py", line 125, in run_job
    retval = job.func(*job.args, **job.kwargs)
  File "/usr/local/lib/python3.10/site-packages/elastalert/elastalert.py", line 1312, in handle_config_change
    self.load_rule_changes()
  File "/usr/local/lib/python3.10/site-packages/elastalert/elastalert.py", line 1128, in load_rule_changes
    new_rule_hashes = self.rules_loader.get_hashes(self.conf, self.args.rule)
  File "/usr/local/lib/python3.10/site-packages/elastalert/loaders.py", line 577, in get_hashes
    rule_mod_times[rule_file] = self.get_rule_file_hash(rule_file)
  File "/usr/local/lib/python3.10/site-packages/elastalert/loaders.py", line 610, in get_rule_file_hash
    rule_file_hash += self.get_rule_file_hash(import_rule_file)
TypeError: can't concat str to bytes"

Checklist

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

Comments

RulesLoader should really be immutable. The fact that it's stateful and caches import_rules is sketchy for updates.

@@ -606,12 +606,14 @@ def get_import_rule(self, rule):
return expanded_imports

def get_rule_file_hash(self, rule_file):
rule_file_hash = ''
Copy link
Contributor Author

@JeffAshton JeffAshton Nov 22, 2021

Choose a reason for hiding this comment

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

Was returning string when file not found, which when concatenated with bytes in the case of import rules,

rule_file_hash += self.get_rule_file_hash(import_rule_file)

throws TypeError: can't concat str to bytes

@JeffAshton JeffAshton force-pushed the get_rule_file_hash-fix-return-type branch from caa0de3 to accc8c7 Compare November 22, 2021 15:53
Copy link
Owner

@jertel jertel left a comment

Choose a reason for hiding this comment

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

Thanks!

@jertel jertel merged commit f6fb434 into jertel:master Nov 22, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 20, 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