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

add tag formatting docs #2426

Merged
merged 3 commits into from
Mar 12, 2019
Merged

add tag formatting docs #2426

merged 3 commits into from
Mar 12, 2019

Conversation

replay
Copy link
Collaborator

@replay replay commented Feb 12, 2019

Defining which characters are allowed, and how to query for values containing quotes.

If you agree with the rules defined in this change, then I can also add an according validator to the TagDB

@codecov-io
Copy link

codecov-io commented Feb 12, 2019

Codecov Report

Merging #2426 into master will decrease coverage by 0.05%.
The diff coverage is 52.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2426      +/-   ##
==========================================
- Coverage   80.11%   80.05%   -0.06%     
==========================================
  Files          85       85              
  Lines        8910     8926      +16     
  Branches     1906     1912       +6     
==========================================
+ Hits         7138     7146       +8     
- Misses       1504     1508       +4     
- Partials      268      272       +4
Impacted Files Coverage Δ
webapp/graphite/tags/utils.py 79.41% <52.94%> (-9.05%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ce7893...7149908. Read the comment docs.

@replay
Copy link
Collaborator Author

replay commented Feb 12, 2019

Added a basic validator:

(graphite-web) mst@mst-nb1:~/documents/code/go/src/github.com/graphite-project/graphite-web/webapp$ python 
Python 2.7.15rc1 (default, Nov 12 2018, 14:31:15) 
[GCC 7.3.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from graphite.tags import utils
>>> utils.TaggedSeries.validateCharacters('abc', 'cba~')
False
>>> utils.TaggedSeries.validateCharacters('abc', 'c;ba')
False
>>> utils.TaggedSeries.validateCharacters('ab!c', 'cba')
False
>>> utils.TaggedSeries.validateCharacters('a^bc', 'cba')
False
>>> utils.TaggedSeries.validateCharacters('a=bc', 'cba')
False
>>> utils.TaggedSeries.validateCharacters('abc', 'cb=a')
True
>>> utils.TaggedSeries.validateCharacters('abc', 'cba')
True

@replay
Copy link
Collaborator Author

replay commented Feb 13, 2019

Just some more testing, by actually running graphite and calling the API:

mst@mst-nb1:~$ curl -XPOST 'localhost:8000/tags/tagSeries' --data-urlencode 'path=disk.used;rack=a1'
"disk.used;rack=a1"
mst@mst-nb1:~$ curl -XPOST 'localhost:8000/tags/tagSeries' --data-urlencode 'path=disk.used;rack=a1~'
{"error": "Tag/Value contains invalid characters: rack/a1~"}
mst@mst-nb1:~$ curl -XPOST 'localhost:8000/tags/tagSeries' --data-urlencode 'path=disk.used;rac!k=a1'
{"error": "Tag/Value contains invalid characters: rac!k/a1"} 
mst@mst-nb1:~$ curl -XPOST 'localhost:8000/tags/tagSeries' --data-urlencode 'path=disk.used;rack^=a1'
{"error": "Tag/Value contains invalid characters: rack^/a1"} 
mst@mst-nb1:~$ curl -XPOST 'localhost:8000/tags/tagSeries' --data-urlencode 'path=disk.used;rack====a1'
"disk.used;rack====a1"
mst@mst-nb1:~$ curl -XPOST 'localhost:8000/tags/tagSeries' --data-urlencode 'path=disk.used;rack=~a1'
{"error": "Tag/Value contains invalid characters: rack/~a1"}

@deniszh
Copy link
Member

deniszh commented Feb 19, 2019

LGTM. Should I merge it, @DanCech ?

@DanCech DanCech merged commit 9066d5a into master Mar 12, 2019
deniszh added a commit that referenced this pull request Oct 23, 2019
…pr-2425_pr-2426_pr-2426_pr-2426_pr-2431_pr-2433_pr-2436_pr-2436_pr-2443_commit-91ed1c0b_pr-2450_pr-2450_pr-2451_pr-2452_pr-2452_pr-2462_pr-2462_pr-2463_pr-2464_pr-2464_pr-2466_pr-2467_

[1.1.x] set package long description (#2407) | fix dashboard graph metric list icon paths with URL_PREFIX (#2424) | docs: for sql db migration to 1.1 recommend --fake-initial (#2425) | add tag formatting docs (#2426) | Update tags.rst (#242
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants