Skip to content

Fixes AzureRM create_object_model util#56379

Merged
Ch3LL merged 8 commits intosaltstack:masterfrom
Ajnbro:fix-create-object
Apr 15, 2020
Merged

Fixes AzureRM create_object_model util#56379
Ch3LL merged 8 commits intosaltstack:masterfrom
Ajnbro:fix-create-object

Conversation

@Ajnbro
Copy link
Copy Markdown
Contributor

@Ajnbro Ajnbro commented Mar 14, 2020

What does this PR do?

This PR fixes the Microsoft Azure Resource Manager create_object_model util. Currently, the util is not properly building some object models. The issue is caused by the util improperly handling the existence of parameters because it does not account for the existence of parameters that are set as Boolean values (specially False).

What issues does this PR fix or reference?

N/A

Previous Behavior

Object models with parameters that may have contained Boolean values (specially False) were being built improperly.

New Behavior

Object models with parameters that may contain Boolean values (specially False) are being built properly.

Tests written?

No

Commits signed with GPG?

Yes

@Ajnbro Ajnbro requested a review from a team as a code owner March 14, 2020 16:23
@ghost ghost requested a review from Ch3LL March 14, 2020 16:23
@Ch3LL Ch3LL self-assigned this Mar 16, 2020
Copy link
Copy Markdown
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

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

can we get some tests added here? and can you create an issue with details of the issue and tie it to this PR, to make it easier for others running into the issue to see it.

@Ch3LL Ch3LL added the needs-testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases label Mar 16, 2020
@nicholasmhughes
Copy link
Copy Markdown
Contributor

@Ch3LL test case added. This issue was actually something that we ran into in Idem, but also could apply to Salt... we just don't have any concrete errors to report.

@Ajnbro Ajnbro requested a review from Ch3LL March 18, 2020 03:26
Ch3LL
Ch3LL previously approved these changes Mar 18, 2020
@Ch3LL Ch3LL added Merge Ready and removed needs-testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases labels Mar 18, 2020
@dwoz dwoz removed the Merge Ready label Apr 14, 2020
@dwoz
Copy link
Copy Markdown
Contributor

dwoz commented Apr 14, 2020

Merge conflicts need to be resolved.

@Ch3LL Ch3LL merged commit 0354421 into saltstack:master Apr 15, 2020
@Ajnbro Ajnbro deleted the fix-create-object branch April 15, 2020 18:49
@sagetherage sagetherage added the ZRelease-Sodium retired label label May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ZRelease-Sodium retired label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants