Skip to content

Conversation

@calvinsID
Copy link
Contributor

Description
Fixes issue #16066

Before, az webapp up/create allowed you to specify --runtime parameter with delimiters "|", " ", and ":".
This parsing caused runtimes with a space (" ") in it to not be parsed correctly.

In this PR, removing ability to use space (" ") as a delimiter so that runtimes with a space in it get parsed correctly again

Testing Guide
az webapp create -n {} -g {} --runtime "'java|1.8|Java SE|8" --os-type Windows should work


This checklist is used to make sure that common guidelines for a pull request are followed.

@panchagnula
Copy link
Contributor

@calvinsID should we remove "" as a delimiter completely or can we just make this stricter.
As in if we have "" & "|" or ":" then we don't assume that " " is a delimiter here but part of the runtime value.

If we see multiple spaces in a case the runtime Value has a space & space is being used as delimiter can we do something to consider the last space in the string to be the delimiter?

@calvinsID
Copy link
Contributor Author

@panchagnula good idea I'll change so that if there are other delimiters first, we ignore the other delimiters. That way if a runtime which has spaces is used, it will work with other delimiters

@panchagnula
Copy link
Contributor

@panchagnula good idea I'll change so that if there are other delimiters first, we ignore the other delimiters. That way if a runtime which has spaces is used, it will work with other delimiters

Sounds good @calvinsID also may be update the help doc to clarify that space is a supported delimiter & add examples on how to use this. Also if a runtime has space in language should't the recommendation be to enclose them in single or double quotes "Java SE" so if the command is used as so

az webapp create -n {} -g {} --runtime "'Java SE"
we don't assume space to be a delimiter for "language version"

@calvinsID
Copy link
Contributor Author

@panchagnula if the runtime has a space in the language it might be messy to allow the use of space as a delimiter, ex. '"java 11 "Java SE" 8"' or '"java 11 Java SE 8"'

@calvinsID calvinsID force-pushed the issue-16066-runtime-space-regression branch from e92aa6a to 7ddb19b Compare January 14, 2021 01:01
@panchagnula
Copy link
Contributor

@panchagnula if the runtime has a space in the language it might be messy to allow the use of space as a delimiter, ex. '"java 11 "Java SE" 8"' or '"java 11 Java SE 8"'

@panchagnula if the runtime has a space in the language it might be messy to allow the use of space as a delimiter, ex. '"java 11 "Java SE" 8"' or '"java 11 Java SE 8"'

Agreed Please update help with some examples with different delimiters & add a test case for command with runtime with language value only like 'Java SE' i.e if we allow runtime version with language value only where we use default version value? otherwise change looks good to me.

@calvinsID
Copy link
Contributor Author

Added examples in the help text. We currently don't allow language value only like "Java SE" though we can add this in a future PR

Copy link
Contributor

@panchagnula panchagnula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for jumping on this fix so quickly from the time I assigned the issue to you :)

@qwordy qwordy merged commit 82d9063 into Azure:dev Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

webapp:Unable to select a runtime version if it includes "space" in its name such as "Java SE"

3 participants