feat: add built-in interval entity #1179
Conversation
|
@kkedziak-splunk some UI tests are failing, please take a look |
|
@artemrys it was a typo, I fixed it on Friday |
artemrys
changed the title
|
Please add a description with an explanation of what was done in this PR, how are existing customers impacted. Do we migrate existing TAs to this new interval field? |
| { | ||
| "type": "regex", | ||
| "errorMsg": f"{self['label']} must be either a non-negative number or -1.", | ||
| "pattern": r"^(?:-1|0(?:\.\d+)?|[1-9]\d*(?:\.\d+)?)$", |
There was a problem hiding this comment.
This regex doesn't work with a value like 00123. Users ideally don't put values like this, but it would be good if we can handle it.
Also, should we have any maximum value? The modinput should at least run once a year or once in six months, something like that?
We can use regex and range validators for this case.
There was a problem hiding this comment.
If there are no limits for intervals in Splunk, I don't think we should come up with some default value.
There are min and max fields in the options so developers can set it.
There was a problem hiding this comment.
I will add leading 0's to the regex.
tests/unit/test_schema.py
Outdated
| "-10", | ||
| "-1.0", | ||
| "-0.1", | ||
| "01", |
There was a problem hiding this comment.
Should 01 be a valid value?
|
Please use the new PR template, you can find it in .github folder. Can you please expand on the user experience section? |
# Conflicts: # splunk_add_on_ucc_framework/commands/build.py # tests/smoke/test_ucc_build.py
| "$ref": "#/definitions/IntervalValue" | ||
| }, | ||
| "options": { | ||
| "anyOf": [ |
There was a problem hiding this comment.
that anyOf seems unnecessary , as you passed only one possibility
There was a problem hiding this comment.
You are right. I forgot to remove it.
Ignore as it should work similarly to logginTab change, so UI code is not affected eventually |
| } | ||
|
|
||
| for name in ("help", "required", "defaultValue", "tooltip"): | ||
| if name in definition: |
There was a problem hiding this comment.
Can you please add a test that covers this line?
Issue number: ADDON-68645
Summary
Changes
This PR adds a new entity type:
interval.It is a text field used for providing numeric value for intervals, i.e. -1, 0 or a positive number (integer or float).
Regex:
^(?:-1|\d+(?:\.\d+)?)$The old intervals added as text fields are migrated to the new solution. There may be a difference in the regex used to check the value - the popular solution allowed to input all negative numbers, while it should not be allowed.
User experience
Although the old interval will be migrated automatically during the build step, the user experience should be the same. The UI will not change. However, the regex validation is a bit different - currently some TA's allow specifying incorrect interval values.
Checklist
If your change doesn't seem to apply, please leave them unchecked.