-
Notifications
You must be signed in to change notification settings - Fork 307
extending regex to match multiple subsequent words in datasource placeholder #53
extending regex to match multiple subsequent words in datasource placeholder #53
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
tasks/dashboards.yml
Outdated
# 4. A one-or-more case-sensitive match for the part that follows the | ||
# underscore, with no characters allowed except alphabetical. | ||
# 3. A case-sensitive literal match for DS . | ||
# 4. One or more case-sensitive matches for groups of alphabetical characters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the trailing whitespace here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the trailing whitespace and the test should pass. Otherwise this LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the two commits are a single change, could you squash and force push up. With that done I think we're good to go here.
placeholder which is slightly more complex than what the regex currently supports. As such we are extending the regex to allow more than just one word following the "DS_" prefix. We do this by following the following rules: - Literal boundaries for " on either side of the match. - Non-capturing optional group matches for the ${} bits which may, or may not, be there.. - A case-sensitive literal match for DS . - One or more case-sensitive matches for groups of alphabetical characters where each group is preceded by an underscore.
5fd90d7
to
23f78f5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
thanks :) |
In order to match datasources like "DS_DML-PROM0" which include dashes and numbers, we open the match to include them. We also remove the capture groups implemented in [1] as they're not required and their inclusion makes things more difficult to understand. Instead, we provide the same function by simply also allowing underscores in the match. [1] cloudalchemy#53
In order to match datasources like "DS_DML-PROM0" which include dashes and numbers, we open the match to include them. We also remove the capture groups implemented in [1] as they're not required and their inclusion makes things more difficult to understand. Instead, we provide the same function by simply also allowing underscores in the match. [1] cloudalchemy#53 Fixes cloudalchemy#59
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
It was discovered that some grafana dashboards use a datasource
placeholder which is slightly more complex than what the regex
currently supports. As such we are extending the regex to
allow more than just one word following the "DS_" prefix.
We do this by following the following rules:
may not, be there..
where each group is preceded by an underscore.