-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[DO NOT MERGE] Fix #10354 #10389
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
[DO NOT MERGE] Fix #10354 #10389
Conversation
src/azure-cli/azure/cli/command_modules/resource/tests/latest/test_resource.py
Outdated
Show resolved
Hide resolved
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 suggest --handle-extended-json-format, -j with help text calling out we will deal with comments, multi-line strings. To me, send unmodified isn't some detail that end users would care. Please correct me if I am wrong.
CC: @alex-frankel
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.
works for me
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.
Sounds good! I prefer adding --handle-extended-json-format to validate command at the same time. What about your opinion?
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.
agreed
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.
agreed
Given validate command is in another custom function, I think we can leave it in next sprint. Because it may be better if we can investigate the behavior in create command after this release and if no problem, I can add it in validate command and I also have enough time to do that.
|
@Juliehzl please merge this PR before this sprint build. |
|
@yugangw-msft Have some changes after your review. If it is good to merge, you can help me merge it or I will merge it. |
|
When will this get released? |
|
|
||
| minified = jsmin(template) | ||
| # Get rid of multi-line strings. Note, we are not sending it on the wire rather just extract parameters to prompt for values | ||
| result = re.sub(r'"[^"]*?\n[^"]*?(?<!\\)"', '"#Azure Cli#"', minified, re.DOTALL) |
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.
re.DOTALL changes the behavior of .:
https://docs.python.org/3/library/re.html#re.DOTALL
re.DOTALL
Make the '.' special character match any character at all, including a newline; without this flag, '.' will match anything except a newline. Corresponds to the inline flag (?s).
But there is no . in the regex at all.
|
|
||
| class JsonCTemplate(object): | ||
| def __init__(self, template_as_bytes): | ||
| self.template_as_bytes = template_as_bytes |
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.
The name template_as_bytes is misleading.
I debugged test_resource.DeploymentTest.test_group_deployment, and saw its value is actually a str:
(I found this when reviewing #21220.)

This checklist is used to make sure that common guidelines for a pull request are followed.
The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).
I adhere to the Command Guidelines.