A number is a valid service name #1855
Merged
+54
−8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I was thinking the problem is that yaml treats a number as an integer, instead of a string. So the developer should probably write: '2':
...I'm not sure about casting these to strings. Maybe we could just warn the developer of the type and have them fix the yaml? |
|
@dnephin Yeh I was in two minds about it myself, partly because I didn't want to force the user to do extra work for something that feels like it should work but then conflicted of casting everything to a string 🎱 Thinking about it some more and talking it over with @aanand, I think you're right and this is a good suggestion, thanks. Importantly with your approach we can always change our minds later, if we go with cast all to strings, we can't as easily change track. |
mnowster
changed the title
mnowster
force-pushed
the
number-is-a-valid-service-name
branch
from
b15675e to
e0906a2
Compare
In order to validate a service name that has been specified as an integer we need to run that as a pre-process validation step *before* we pass the config to be validated against the schema. It is not possible to validate it *in* the schema, it causes a type error. Even though a number is a valid service name, it must be a cast as a string within the yaml to avoid type error. Taken this opportunity to move the code design in a direction towards: 1. pre-process 2. validate 3. construct Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
mnowster
force-pushed
the
number-is-a-valid-service-name
branch
from
e0906a2 to
67995ab
Compare
mnowster
changed the title
|
LGTM |
aanand
added a commit
that referenced
this pull request
Aug 20, 2015
A number is a valid service name
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While we technically allow a service name to be a number, it was causing the code
to blow up with a TypeError, as reported #1845.
This is due to it expecting a string or a buffer. So I took the approach to ensure all
service names get converted to strings.
Signed-off-by: Mazz Mosley mazz@houseofmnowster.com