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

[16.04] Rev Pulsar to 0.7.0.dev3. #2122

Merged
merged 1 commit into from Apr 12, 2016

Conversation

jmchilton
Copy link
Member

  • Fixes numerous problems mostly notably fixes external metadata generation and job metrics collection for recent Pulsar. This was broken when tools were given clean working directory.
  • Produce a new message if an older Pulsar version is targeted via REST of library (should help with upgrading to Galaxy 16.04).
  • Newer pulsar lib allows more flexible configuration of managers - update documentation to reflect this.

 - Fixes numerous problems mostly notably fixes external metadata generation and job metrics collection for recent Pulsar. This was broken when tools were given clean working directory.
 - Produce a new message if an older Pulsar version is targetted via REST of library (should help with upgrading to Galaxy 16.04).
 - Newer pulsar lib allows more flexible configuration of managers - update documntation to reflect this.
@@ -205,6 +206,14 @@ def __init_pulsar_app( self, pulsar_conf_path ):
log.info("Loading Pulsar app configuration from %s" % pulsar_conf_path)
with open(pulsar_conf_path, "r") as f:
conf.update(yaml.load(f) or {})
if "job_metrics_config_file" not in conf:
conf["job_metrics"] = self.app.job_metrics
Copy link
Member

Choose a reason for hiding this comment

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

job_metrics or job_metrics_config_file ?

Copy link
Member Author

Choose a reason for hiding this comment

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

job_metrics is intentional here. If the user doesn't specify an explicit configuration file for Pulsar that is different - I just pass through the actual object (job_metrics) through to Pulsar and it re-uses it - instead of constructing a new one from a configuration file.

Copy link
Member

Choose a reason for hiding this comment

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

I was wondering because in the if you check for "job_metrics_config_file"

Copy link
Member Author

Choose a reason for hiding this comment

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

job_metrics_config_file in conf would mean the deployer specified a file explicitly and so that should be respected. Hence if "job_metrics_config_file" not in conf then a file wasn't specified and the conf object can just be updated to inject Galaxy's job_metrics object directly. I think this correct.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, ok, that's because Pulsar supports both configuration options (xref https://github.com/galaxyproject/pulsar/blob/master/pulsar/core.py#L129 ). Works for me.

@natefoo natefoo merged commit f03c037 into galaxyproject:release_16.04 Apr 12, 2016
@natefoo
Copy link
Member

natefoo commented Apr 15, 2016

@jmchilton If I understand correctly, this does not make it possible to have per-manager or per-destination staging directories, correct?

@jmchilton
Copy link
Member Author

jmchilton commented Apr 15, 2016

@natefoo - Umm... still not yet I guess but you should only need one pulsar app/plugin for many destinations/managers now right? Do you still want per-manager staging directories?

@natefoo
Copy link
Member

natefoo commented Apr 15, 2016

Gonna need them to choose between IU and TACC Jetstreams because they have different staging dirs for each. But it's fine, it works as is with two plugins.

@jmchilton
Copy link
Member Author

I could be missing something - but it is probably as easy as galaxyproject/pulsar#104? With no changes needed on the Galaxy side? Any interest in testing this? I can cut a new version if it looks good.

@natefoo
Copy link
Member

natefoo commented Apr 19, 2016

For anyone reading in the future, this does indeed work with galaxyproject/pulsar#104 and #2184.

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

Successfully merging this pull request may close these issues.

None yet

3 participants