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

Use safe_eval instead of temporary rules for comparison #241

Merged
merged 10 commits into from
Feb 3, 2023

Conversation

geof77
Copy link
Contributor

@geof77 geof77 commented Jan 24, 2023

In PR #233 we create temporary rules as a means to get the exact API representation for the value_raw.

Ansible's module_utils library provides a "safe_eval()" implementation of ast.literal_eval(). As it was suggested by a developer in #153 we could use this technique to evaluate the values as Python objects before the comparison, as other modules and checkmk api does.

This removes the need to create a temporary rule.

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Create temp rule
Fill the logs
Slow

Issue Number: #153 #186

What is the new behavior?

Use safe_eval from ansible module_utils
Does not fill the logs
Faster

@geof77 geof77 requested a review from lgetwan as a code owner January 24, 2023 23:25
@geof77 geof77 marked this pull request as draft January 24, 2023 23:35
@lgetwan lgetwan self-assigned this Jan 25, 2023
@lgetwan lgetwan added enhancement New feature or request module:rule This affects the rule module labels Jan 25, 2023
@lgetwan
Copy link
Contributor

lgetwan commented Jan 25, 2023

This looks promising!

@geof77 geof77 marked this pull request as ready for review January 25, 2023 08:03
Copy link
Contributor

@lgetwan lgetwan left a comment

Choose a reason for hiding this comment

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

Successfully tested

@lgetwan lgetwan merged commit 4178145 into Checkmk:devel Feb 3, 2023
@robin-checkmk robin-checkmk mentioned this pull request Feb 3, 2023
7 tasks
robin-checkmk pushed a commit that referenced this pull request May 22, 2023
Use safe_eval instead of temporary rules for comparison
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request module:rule This affects the rule module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants