Skip to content
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

Fix virt.get_hypervisor() #55351

Merged
merged 1 commit into from
Nov 25, 2019
Merged

Conversation

cbosdo
Copy link
Contributor

@cbosdo cbosdo commented Nov 18, 2019

What does this PR do?

virt.get_hypervisor resulted in:

AttributeError: module 'salt.loader.dev-srv.tf.local.int.module.virt' has no attribute '_is_{}_hyper'

This was due to missplaced parenthese.

What issues does this PR fix or reference?

No issue filed for this yet.

Previous Behavior

calling virt.get_hypervisor was throwing the error:

AttributeError: module 'salt.loader.dev-srv.tf.local.int.module.virt' has no attribute '_is_{}_hyper'

New Behavior

calling virt.get_hypervisor now returns the actual value

Tests written?

No - would require to abstract the virt._is_*_hyper functions and thus only test one line of rather simple code... even though that one contained a misplaced parenthese.

Commits signed with GPG?

Yes

@cbosdo cbosdo requested a review from a team as a code owner November 18, 2019 17:27
@ghost ghost requested a review from Ch3LL November 18, 2019 17:27
@Akm0d Akm0d added Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases labels Nov 20, 2019
@Akm0d Akm0d added this to the Approved milestone Nov 20, 2019
@Akm0d
Copy link
Contributor

Akm0d commented Nov 20, 2019

Even for a simple change like moving a parenthesis a unit test is necessary so that we don't have regressions. I can help whip that up when I'm done triaging for the day 👍

virt.get_hypervisor resulted in:

  AttributeError: module 'salt.loader.dev-srv.tf.local.int.module.virt' has no attribute '_is_{}_hyper'

This was due to missplaced parenthese.
@cbosdo
Copy link
Contributor Author

cbosdo commented Nov 20, 2019

Even for a simple change like moving a parenthesis a unit test is necessary so that we don't have regressions. I can help whip that up when I'm done triaging for the day +1

Done... not sure the test is super useful though ;)

@Akm0d Akm0d removed the Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases label Nov 20, 2019
@Akm0d
Copy link
Contributor

Akm0d commented Nov 20, 2019

Thank you so much!

@Akm0d
Copy link
Contributor

Akm0d commented Nov 20, 2019

re-run full macosxmojave

@Akm0d
Copy link
Contributor

Akm0d commented Nov 20, 2019

re-run full amazon1

@Akm0d
Copy link
Contributor

Akm0d commented Nov 22, 2019

re-run full tornado

@dwoz dwoz merged commit a225a3f into saltstack:master Nov 25, 2019
@cbosdo cbosdo deleted the master-virt-hypervisor-fix branch November 26, 2019 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Confirmed Salt engineer has confirmed bug/feature - often including a MCVE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants