Skip to content
This repository has been archived by the owner on Feb 10, 2021. It is now read-only.

Flesh out doctstring for DRMAACluster #74

Merged
merged 2 commits into from
Apr 27, 2018
Merged

Conversation

edurand
Copy link
Contributor

@edurand edurand commented Apr 6, 2018

As discussed in issue 72, I fleshed out the docstring for DRMAACluster. Hopefully it is now a little bit easier to use.

args: list
Extra string arguments to pass to dask-worker
outputPath: string
Path to the dask-worker stdout. Must start with ':'.
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not understand why a path would need to start with ":"

Copy link
Member

Choose a reason for hiding this comment

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

So don't think the : is necessary, but can be used to specify a file on a different host by prefacing with a hostname or IP IIRC.

Copy link
Member

Choose a reason for hiding this comment

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

FWIW normally I'll do something like localhost:my_job.out. Similar story with errorPath.

Copy link
Contributor

@maxnoe maxnoe left a comment

Choose a reason for hiding this comment

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

Could you also explain how outputPath and errorPath could be used?

I don't get why a path needs to start with : and how to use templating with the JOB_ID or similar.

@edurand
Copy link
Contributor Author

edurand commented Apr 13, 2018

I believe the ":" is required by DRMAA to be considered a valid path. I am unsure why.

One possibility would be to allow the user to pass an ouputTemplate and errorTemplate instead of outputPath and errorPath. Then the docstring might look something like

outputTemplate: Template to specify job output path. Must be formatted as 'basename.{jid}.out'. {jid} will be automatically replaced by JOBID.JOBINDEX to avoid multiple jobs writing to the same file. For instance, to have your jobs output to files with basenames /scratch/job_output, specify outputTemplate as "/scratch/job_output.{jid}.out".

How does that look?

@maxnoe
Copy link
Contributor

maxnoe commented Apr 13, 2018

I like it

@maxnoe
Copy link
Contributor

maxnoe commented Apr 20, 2018

I think it would be great to go ahead with this doc fix and make a new release with the other recent changes.

@jakirkham
Copy link
Member

@edurand, do you have time to fix the merge conflicts?

@jakirkham jakirkham merged commit 67d90aa into dask:master Apr 27, 2018
@jakirkham
Copy link
Member

Based on your recent comments @maxnoe, I'm assuming you are happy with this, but just noticed there was an X in the review section. Was there anything else we need to address?

@jakirkham
Copy link
Member

Also thanks @edurand for the contribution. The doc refresh was definitely needed. :)

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

Successfully merging this pull request may close these issues.

3 participants