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

Update threshold rule schema to disallow empty field string #1098

Merged

Conversation

brokensound77
Copy link
Contributor

@brokensound77 brokensound77 commented Apr 12, 2021

Issues

related to #1097
related to #1099 (same change targeting main)

Summary

Changes threshold.field to disallow empty strings and instead just use an empty array for non-defined values.

@brokensound77 brokensound77 added Rule: Tuning tweaking or tuning an existing rule python Internal python for the repository v7.12.1 labels Apr 12, 2021
@brokensound77 brokensound77 requested a review from rw-access April 12, 2021 14:40
@@ -15,6 +15,7 @@

DATE_PATTERN = r'\d{4}/\d{2}/\d{2}'
MATURITY_LEVELS = ['development', 'experimental', 'beta', 'production', 'deprecated']
NON_EMPTY_STR = r'.+'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
NON_EMPTY_STR = r'.+'

@@ -31,7 +32,7 @@ class ThresholdCardinality(jsl.Document):
field = jsl.StringField(required=True)
value = jsl.IntField(minimum=1, required=True)

field = jsl.ArrayField(jsl.StringField(required=True, default=""))
field = jsl.ArrayField(jsl.StringField(required=False, pattern=NON_EMPTY_STR), required=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
field = jsl.ArrayField(jsl.StringField(required=False, pattern=NON_EMPTY_STR), required=True)
field = jsl.ArrayField(jsl.StringField(required=False, min_length=1), required=True)

Copy link
Contributor

@rw-access rw-access left a comment

Choose a reason for hiding this comment

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

LGTM if you switch to min_length

@brokensound77
Copy link
Contributor Author

Done 👍 . I also locked versions as this should be the last change to 7.12.1

"sha256": "9aeb1b1c1c4b9fb39b564774f25afea20210e5789afbde43998e680892678b7c",
"version": 3
"sha256": "9aee800e271c99dfee70b36fbe34a3607ad2b2c9030f77b26db66977cce7621f",
"version": 4
Copy link
Contributor

Choose a reason for hiding this comment

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

😆 the joys of versioning

Copy link
Contributor

Choose a reason for hiding this comment

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

don't forget to pull this lock into main btw

@brokensound77 brokensound77 merged commit 462fab3 into elastic:7.12 Apr 14, 2021
@brokensound77 brokensound77 deleted the disallow-empty-threshold-field-7.12.1 branch April 14, 2021 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Internal python for the repository Rule: Tuning tweaking or tuning an existing rule v7.12.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants