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

Fixup! add master.py:FileserverUpdate **kwargs #47851

Merged
merged 1 commit into from Jun 12, 2018

Conversation

Projects
None yet
4 participants
@rares-pop
Contributor

rares-pop commented May 26, 2018

Fixup! add master.py:FileserverUpdate **kwargs

All classes inherited from salt/utils/process.py:MultiprocessingProcess
must use **kwargs after commit 90edc69

Signed-off-by: Rares POP rares.pop@ni.com

What does this PR do?

It fixes the master on Windows platform.

What issues does this PR fix or reference?

Previous Behavior

salt-master was broken

Tests written?

No

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

Fixup! add master.py:FileserverUpdate **kwargs
Signed-off-by: Rares POP <rares.pop@ni.com>

@rares-pop rares-pop requested a review from saltstack/team-core as a code owner May 26, 2018

@gtmanfred

One major grip.

Can you also provide us with more information than 'FIXUP! add master.py:FileserverUpdate **kwargs` This doesn't tell us anything on why it is needed or what is "broken"

@@ -349,8 +349,8 @@ class FileserverUpdate(salt.utils.process.SignalHandlingMultiprocessingProcess):
'''
A process from which to update any dynamic fileserver backends
'''
def __init__(self, opts, log_queue=None):
super(FileserverUpdate, self).__init__(log_queue=log_queue)
def __init__(self, opts, **kwargs):

This comment has been minimized.

@gtmanfred

gtmanfred May 29, 2018

Contributor

I do not like the idea of using kwargs for something like this, especially without documenting what can be passed as kwargs.

This comment has been minimized.

@rares-pop

rares-pop May 29, 2018

Contributor

All classes inherited from salt/utils/process.py:MultiprocessingProcess must use **kwargs after commit 90edc69

Basically the windows build is broken now. When you try to start salt-master, it just throws.

This comment has been minimized.

@gtmanfred

gtmanfred May 29, 2018

Contributor

Perfect, thank you for the explanation. Now it makes sense on why you are using **kwargs

This comment has been minimized.

@rares-pop

rares-pop May 29, 2018

Contributor

Sure! I'll add the explanation in the description. Sorry for the misunderstanding.

This comment has been minimized.

@gtmanfred

gtmanfred May 29, 2018

Contributor

Awesome! Thank you very much for this addition!

Daniel

@rallytime rallytime requested review from terminalmage and cachedout May 31, 2018

@terminalmage

@rallytime the FileserverUpdate process was added in 2018.3, so this will need a backport.

@rares-pop

This comment has been minimized.

Contributor

rares-pop commented Jun 12, 2018

As a followup - should I backport this to 2018.3? I can totally do that.

@gtmanfred

This comment has been minimized.

Contributor

gtmanfred commented Jun 12, 2018

It is marked to be backported, we will backport it.

@gtmanfred gtmanfred merged commit e217a17 into saltstack:develop Jun 12, 2018

5 of 10 checks passed

jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #5309 — ABORTED
Details
jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #10280 — ABORTED
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #19365 — ABORTED
Details
default Build finished.
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #23243 — FAILURE
Details
WIP ready for review
Details
codeclimate All good!
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #25499 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #17582 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #22212 — SUCCESS
Details

rallytime added a commit that referenced this pull request Jun 14, 2018

@rares-pop rares-pop deleted the rares-pop:dev/iepopr/fixup-master-py-FileserverUpdate-kwargs branch Jun 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment