-
Notifications
You must be signed in to change notification settings - Fork 88
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 validate_tags
util to check that tags are an array of strings
#218
Conversation
@@ -205,4 +205,13 @@ def Dogapi.find_localhost | |||
raise $ERROR_INFO unless $ERROR_INFO.class.name == 'Errno::ENOENT' | |||
@@hostname = Addrinfo.getaddrinfo(Socket.gethostname, nil, nil, nil, nil, Socket::AI_CANONNAME).first.canonname | |||
end | |||
|
|||
def Dogapi.validate_tags(tags) | |||
unless tags.is_a? Array |
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.
I am not this kind of precious stone expert but what if tags is null?
What calls validate_tags?? |
It will be up to the user to call that utility function in order to validate the tags they pass to the API. |
That seems counter to the original report. |
True this doesn't exactly answer the original report, I'll leave a comment on the issue explaining our reasoning with this. We didn't want to add this validation to every endpoint that manipulates tags because it can cause some performance drop given that we have to check for every individual tag. However, if there is a real need for this, we now at least provide this utility function for a user to easily validate their tags, an for us to later add this to every endpoint. |
What does this PR do?
SSIA
fixes #50
Verification Process
Unit tests
Review checklist (to be filled by reviewers)
changelog/
label attached. If applicable it should have thebackward-incompatible
label attached.do-not-merge/
label attached.kind/
andseverity/
labels attached at least.