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

Remove deprecated bits for Sodium in virt #56414

Merged
merged 15 commits into from
Apr 18, 2020

Conversation

cbosdo
Copy link
Contributor

@cbosdo cbosdo commented Mar 19, 2020

What does this PR do?

Removes all the deprecated parameters, configuration and functions that are to be removed in Sodium for the virt module.

What issues does this PR fix or reference?

Tests written?

Yes: tests adapted where needed

Commits signed with GPG?

Yes

@cbosdo cbosdo requested a review from a team as a code owner March 19, 2020 13:31
@ghost ghost requested a review from DmitryKuzmenko March 19, 2020 13:31
@cbosdo cbosdo force-pushed the virt-na-deprecated-removal branch from 354005b to 53b39cd Compare March 19, 2020 14:15
Copy link
Contributor

@DmitryKuzmenko DmitryKuzmenko left a comment

Choose a reason for hiding this comment

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

LGTM, I'm just having a question about the removed pool argument.

salt/modules/virt.py Outdated Show resolved Hide resolved
@cbosdo
Copy link
Contributor Author

cbosdo commented Apr 2, 2020

@DmitryKuzmenko answered your question and rebased on master

DmitryKuzmenko
DmitryKuzmenko previously approved these changes Apr 2, 2020
@DmitryKuzmenko
Copy link
Contributor

@cbosdo could you please resolve merge conflicts?

@cbosdo
Copy link
Contributor Author

cbosdo commented Apr 7, 2020

rebased

@cbosdo cbosdo force-pushed the virt-na-deprecated-removal branch from 71ee09d to 23feca4 Compare April 7, 2020 09:05
DmitryKuzmenko
DmitryKuzmenko previously approved these changes Apr 7, 2020
@dwoz
Copy link
Contributor

dwoz commented Apr 11, 2020

@cbosdo black needs to be run on this.

@cbosdo
Copy link
Contributor Author

cbosdo commented Apr 12, 2020

OK, I'll rerun it on Tuesday morning

@cbosdo cbosdo force-pushed the virt-na-deprecated-removal branch from 4f9c388 to 61640bd Compare April 14, 2020 07:24
@cbosdo
Copy link
Contributor Author

cbosdo commented Apr 14, 2020

@cbosdo black needs to be run on this.

@dwoz done

ElementTree.tostring() implementation starts to keep the attribute order
defined by the user in python 3.8. This can lead to equal elements to be
considered different. Use xmlutil.to_dict(element, True) to compare the
elements.

This also uncovered a wrong behavior of pool_update when only changing
the password.
@cbosdo cbosdo force-pushed the virt-na-deprecated-removal branch from 61640bd to e90e78d Compare April 15, 2020 07:13
@cbosdo
Copy link
Contributor Author

cbosdo commented Apr 15, 2020

rebased on latest master. @dwoz could that one be merged since there is only the codecov check that is red for some weird reason?

@waynew
Copy link
Contributor

waynew commented Apr 16, 2020

There was also a windows failure - yeah, at this point codecov is just considered informative, and we've changed that on master, so going forward it shouldn't falsely flag failures.

I re-started the windows2016 job - failures didn't look virt related so I'm suspecting infrastructure flakiness, which we have plans to deal with.

@cbosdo
Copy link
Contributor Author

cbosdo commented Apr 17, 2020

@waynew the windows test went green!

Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

🪓 👍

@dwoz dwoz merged commit ae5af4f into saltstack:master Apr 18, 2020
@cbosdo cbosdo deleted the virt-na-deprecated-removal branch April 20, 2020 06:52
@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