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 chronos client initialization #4120

Merged
merged 3 commits into from Jun 5, 2017

Conversation

Projects
None yet
5 participants
@theosotr
Copy link
Contributor

commented May 29, 2017

Provide the missing keyword arguments during chronos client
initialization based on the runner parameters.

Fix chronos client initialization
Provide the missing keyword arguments during chronos client
initialization based on the runner parameters.

@galaxybot galaxybot added the triage label May 29, 2017

@galaxybot galaxybot added this to the 17.09 milestone May 29, 2017

@jmchilton

This comment has been minimized.

Copy link
Member

commented May 30, 2017

@galaxybot test this

@@ -115,8 +115,13 @@ def __init__(self, app, nworkers, **kwargs):
kwargs[self.RUNNER_PARAM_SPEC_KEY] = {}
kwargs[self.RUNNER_PARAM_SPEC_KEY].update(self.RUNNER_PARAM_SPEC)
super(ChronosJobRunner, self).__init__(app, nworkers, **kwargs)
protocol = 'http' if self.runner_params.get('insecure', True)\

This comment has been minimized.

Copy link
@bgruening

bgruening May 30, 2017

Member

Can you put this into one line? We don't follow the 80vchars pep8 rule :)

This comment has been minimized.

Copy link
@theosotr

theosotr May 30, 2017

Author Contributor

OK

self._chronos_client = chronos.connect(
self.runner_params['chronos'])
self.runner_params['chronos'],
username=self.runner_params.get('username'),

This comment has been minimized.

Copy link
@bgruening

bgruening May 30, 2017

Member

What happens if the username and password is not set? Should we default here so something? .get('username', None)

This comment has been minimized.

Copy link
@theosotr

theosotr May 30, 2017

Author Contributor

get(key) returns None by default, if the key is not found, so there is no problem. :)

@dannon

This comment has been minimized.

Copy link
Member

commented May 31, 2017

@theosotr Can you add the 'insecure' option to job_conf.xml.sample_advanced? Then I think this is good to go.

@jmchilton jmchilton merged commit 294c1d1 into galaxyproject:dev Jun 5, 2017

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.