Skip to content

Fixes AzureRM create_object_model util #56379

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

Merged
merged 8 commits into from
Apr 15, 2020
Merged

Fixes AzureRM create_object_model util #56379

merged 8 commits into from
Apr 15, 2020

Conversation

Ajnbro
Copy link
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
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
Collaborator

@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
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.

5 participants