Skip to content
This repository has been archived by the owner on May 31, 2023. It is now read-only.

dashboards: Make the regex much more specific #45

Merged
merged 1 commit into from
Mar 25, 2018

Conversation

odyssey4me
Copy link
Collaborator

@odyssey4me odyssey4me commented Mar 22, 2018

To make sure that we're not being too greedy with the regex
replacement of the data source to use for each dashboard
that's uploaded, we make it much more specific using the
following:

  • literal boundaries for " on either side of the match
  • non-capturing optional group matches for the ${} bits
  • the case-sensitive literal match for DS_ which is
    always in every exported dashboard representing the
    data source used
  • a one-or-more case-sensitive match for the part that
    follows the underscore, with no characters allowed
    except alphabetical

This should prevent mistaking any capitalised labels or
anything else which may be present in the dashboard.

This regex can be tested and understood better by looking at the
matches and non-matches in https://regex101.com/r/f4Gkvg/2
versus the current regex and same data set shown in
https://regex101.com/r/Ad9f5U/1

@paulfantom
Copy link
Member

Please write a comment before task or a better task name explaining what those hieroglyphs do. I fear we might forget and this part will become unmaintainable.

@odyssey4me
Copy link
Collaborator Author

:) will do

To make sure that we're not being too greedy with the regex
replacement of the data source to use for each dashboard
that's uploaded, we make it much more specific using the
following:

- literal boundaries for " on either side of the match
- non-capturing optional group matches for the ${} bits
- the case-sensitive literal match for DS_ which is
  always in every exported dashboard representing the
  data source used
- a one-or-more case-sensitive match for the part that
  follows the underscore, with no characters allowed
  except alphabetical

This should prevent mistaking any capitalised labels or
anything else which may be present in the dashboard.
@odyssey4me odyssey4me force-pushed the more-restrictive-regex branch from d2e3fa5 to 251c7a2 Compare March 22, 2018 20:39
@paulfantom paulfantom self-requested a review March 22, 2018 23:55
@paulfantom paulfantom merged commit 6601e72 into cloudalchemy:master Mar 25, 2018
@aman0019
Copy link

dahsboards such as these:
https://grafana.com/dashboards/2263
https://grafana.com/dashboards/1234

use a datasource placeholder which does not fit the pattern expected by the regex (in this case DS_NDF_APP ).

Should this be reconsidered?

@aman0019
Copy link

this would fix it: "(?:${)?DS(_([A-Z])+)+(?:})?" by allowing one or more words delimited by underscores

@odyssey4me odyssey4me deleted the more-restrictive-regex branch April 25, 2018 22:09
@odyssey4me
Copy link
Collaborator Author

Ah, I wasn't aware that additional underscores were an option. Could you push up a PR and add that example into the regex tester reference (and update it)? Alternatively, let me know and I'll push up a patch with the changes.

@aman0019
Copy link

PR: #53
Updated regex: https://regex101.com/r/f4Gkvg/4

:)

@odyssey4me
Copy link
Collaborator Author

Excellent, thanks!

@lock
Copy link

lock bot commented Mar 24, 2019

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.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019
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.

3 participants