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

Changed glusterfs.peer() module so state can handle localhost peering attempts. #30987

Merged
merged 2 commits into from Feb 9, 2016
Merged

Changed glusterfs.peer() module so state can handle localhost peering attempts. #30987

merged 2 commits into from Feb 9, 2016

Conversation

youngnick
Copy link

When the _gluster_xml command-calling method was introduced, glusterfs.peer() lost the ability to pass the output string back to the calling function. This meant that, in the case that you were attempting to peer probe localhost, glusterfs.peer would return successfully, the state would populate its .comment return value with a boolean value, and the output formatters would not be able to cast bool correctly, making the output look different. Not a functional regression, just a display one.

This just changes glusterfs.peer() to return a dict with the return code and output - possibly overkill since it's not used anywhere, but it's a little nicer for command-line execution now as well. I couldn't find any other references to return codes anywhere else, but if people were relying on the return val of the module being boolean, this would break that.

I think having the command exitval is more useful, personally.

…ocalhost peering attempts.

When the _gluster_xml command-calling method was introduced, glusterfs.peer() lost the ability to pass the output string back to the calling function. This meant that, in the case that you were attempting to peer probe localhost, glusterfs.peer would return successfully, the state would populate its .comment return value with a boolean value, and the output formatters would not be able to cast bool correctly, making the output look different. Not a functional regression, just a display one.

This just changes glusterfs.peer() to return a dict with the return code and output - possibly overkill since it's not used anywhere, but it's a little nicer for command-line execution now as well. I couldn't find any other references to return codes anywhere else, but if people were relying on the return val of the module being boolean, this would break that.

I think having the command exitval is more useful.
@youngnick
Copy link
Author

Okay, will fix tests and come back.

@cachedout
Copy link
Contributor

@youngnick Sounds good. Please let me know when this is ready to go. Thanks!

@cachedout cachedout added the pending-changes The pull request needs additional changes before it can be merged label Feb 8, 2016
@youngnick
Copy link
Author

@cachedout Okay tests are passing now - I added some tests to cover different return cases from the peer probe process.

cachedout pushed a commit that referenced this pull request Feb 9, 2016
…ling

Changed glusterfs.peer() module so state can handle localhost peering attempts.
@cachedout cachedout merged commit c3f1157 into saltstack:2015.8 Feb 9, 2016
@cachedout
Copy link
Contributor

Thanks, @youngnick . Much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending-changes The pull request needs additional changes before it can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants