Skip to content

Add a subprocess job - #50

Open
dlecocq wants to merge 4 commits into
masterfrom
dan/bash-job
Open

Add a subprocess job#50
dlecocq wants to merge 4 commits into
masterfrom
dan/bash-job

Conversation

@dlecocq

@dlecocq dlecocq commented Oct 24, 2016

Copy link
Copy Markdown

@neilmb @b4hand @lindseyreno -- this is what we'd discussed recently. I have mixed feelings about it, but as long as it's run under a service account, I suppose I don't see the problem.

@neilmb neilmb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does this mean that our queue has to be called process? Or are we meant to subclass this with a new method of the proper queue name that forwards on to self.process?

@dlecocq

dlecocq commented Oct 24, 2016

Copy link
Copy Markdown
Author

No, failing the presence of a queue-named method, qless will also check for the presence of a generic process method. So this means we can put it in any queue we want.

Comment thread qless/jobs.py
from . import logger


class SubprocessJob(object):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure I would make this directly available in qless-py. I think it makes sense for our described use case, and if someone wants to do the same thing, it's not that hard for them to reproduce this work, but I wouldn't want to open every qless install up to this.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah... that's when my spidey sense began to activate. I could see myself using this pretty regularly, but for the folks out there who decided to run the thing as root...

Any ideas of how else to distribute it? A separate package?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FWIW, even without this job, I think the privileged user comment in the README should stand just as a cautionary reminder.

I would be fine as a separate package. I would point out that this Job is essentially shell access for the qless worker user for any remote client that can submit jobs.

@dlecocq
dlecocq force-pushed the dan/bash-job branch 4 times, most recently from 5960c6a to 5c43869 Compare October 26, 2016 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants