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 runner parameter names in newer salt version (Boron) #51

Merged
merged 1 commit into from Nov 28, 2016

Conversation

rjfd
Copy link
Contributor

@rjfd rjfd commented Nov 25, 2016

The first parameter of a runner function cannot be named "name",
as it is used internally by salt.

Fixes: #50

Signed-off-by: Ricardo Dias rdias@suse.com

@rjfd rjfd added the bug label Nov 25, 2016
@rjfd
Copy link
Contributor Author

rjfd commented Nov 25, 2016

@jan--f Can you test this please.

"""
Check a cluster for runtime configurations that may cause issues for an
installation.
"""
if name == None:
name = kwargs['name']
if cluster_name == None:
Copy link
Contributor

Choose a reason for hiding this comment

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

let's make this cluster_name is None

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@rjfd rjfd force-pushed the wip-fix-boron-runner-param-error branch from 12acf22 to 41bb8d4 Compare November 25, 2016 16:35
@jan--f
Copy link
Contributor

jan--f commented Nov 28, 2016

Works for me under salt 2016.3.4 (Boron). @supriti can you check if it works for you now too please.

Maybe just 'cluster' would be better as the parameter? This would keep things consistent with the pillar data in DeepSea.

@rjfd
Copy link
Contributor Author

rjfd commented Nov 28, 2016

@jan--f you're right. Will change it to cluster only.

The first parameter of a runner function cannot be named "name",
as it is used internally by salt.

Fixes: #50

Signed-off-by: Ricardo Dias <rdias@suse.com>
@rjfd rjfd force-pushed the wip-fix-boron-runner-param-error branch from 41bb8d4 to b316da1 Compare November 28, 2016 09:34
@rjfd
Copy link
Contributor Author

rjfd commented Nov 28, 2016

@jan--f pushed the rename changes. Might be worth to test again to check if I didn't screwed up anything the with the rename.

Copy link
Contributor

@theanalyst theanalyst left a comment

Choose a reason for hiding this comment

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

code looks good, need to validate against Boron to be sure

Copy link
Contributor

@jan--f jan--f left a comment

Choose a reason for hiding this comment

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

Code looks good and it works for me in under Boron and Beryllium. So thumbs up.

@swiftgist swiftgist merged commit 0b07d11 into master Nov 28, 2016
@rjfd rjfd deleted the wip-fix-boron-runner-param-error branch March 24, 2017 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants