Skip to content
This repository was archived by the owner on Sep 16, 2020. It is now read-only.

Allow starting job with job_tags for superadmins#137

Closed
styxit wants to merge 6 commits intoansible:masterfrom
styxit:job-starting-with-tags
Closed

Allow starting job with job_tags for superadmins#137
styxit wants to merge 6 commits intoansible:masterfrom
styxit:job-starting-with-tags

Conversation

@styxit
Copy link

@styxit styxit commented Apr 16, 2016

Allow super admins to start jobs through POST /jobs and GET /jobs/%d/start. Which allows the use of providing tags. For other users, use the template /launch endpoint.

In order to do this, a call to /me had to be made to determine if a user has superuser privileges.

If this is not the way to go, feel free to close this pull request.

See issue: #136

styxit added 2 commits April 17, 2016 01:16
The `/me` call should not be made every time. Only when needed.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 88c1f7d on styxit:job-starting-with-tags into e8ba983 on ansible:master.

@AlanCoding
Copy link
Member

Thanks for the contribution!

I see what you're doing with the /me/ endpoint, and that's fine by me as a means of determining if the user is a superuser. However, this would direct launch commands with the --job-template flag to the /jobs/ launch method. In many cases, the job template will have configuration data that the user is counting on.

My initial thoughts here are to:

  • add a clause to always use the job template launch if the --job-template flag is given
  • if that flag is not given, then proceed with the superuser check logic in this PR

@styxit
Copy link
Author

styxit commented Apr 22, 2016

if that flag is not given, then proceed with the superuser check logic in this PR

In that case, how can a job be created without a job_template if the only parameters for this command are:

  def launch(self, job_template=None, tags=None, monitor=False, timeout=None,
               no_input=True, extra_vars=None, **kwargs):

@AlanCoding
Copy link
Member

Having the job_template parameter without any others is sufficient to launch a job. There's not a problem that other parameters like tags or extra_vars are None in that case.

@rvdh
Copy link

rvdh commented Apr 22, 2016

@AlanCoding

However, this would direct launch commands with the --job-template flag to the /jobs/ launch method.
In many cases, the job template will have configuration data that the user is counting on.

Indeed, that's why the /jobs/ POST call supports a job_template field, to create a new job based on a template, right?

My initial thoughts here are to:

add a clause to always use the job template launch if the --job-template flag is given
if that flag is not given, then proceed with the superuser check logic in this PR

If that flag is not given, you can't create a new job based on a template. So thats not what we're trying to achieve here - we just want to run a job, based on a template, with tags.

@AlanCoding
Copy link
Member

Indeed, that's why the /jobs/ POST call supports a job_template field, to create a new job based on a template, right?

Thanks for bringing this up, I realize that I was wrong about what the intent was. /jobs/ provides a great amount of flexibility for superusers to create jobs on the fly with any amount of over-writing of the Job Template fields.

We still want to default to the /launch/ endpoint, and to enable the old use we need a new scheme to distinguish between the two in a way that is predictable to the user. I will double-check with the docs and come back to this topic.

@AlanCoding
Copy link
Member

We have implemented another way to get this functionality in PR #184. The concern was that logic to deal with a POST to the /jobs/ endpoint will affect jobs launched via a job template as well, so in the new PR, the "old" way of launching jobs is enabled by the use of a flag. This will allow the passing of tags in the launch, as well as other fields.

@AlanCoding
Copy link
Member

AlanCoding commented Aug 3, 2016

We should have this functionality covered, and will be releasing it within the next week or so. Please don't hesitate to ask if you want to know how to do a particular task with tower-cli in that release.

And thanks so much for helping to improve the code base.

@AlanCoding AlanCoding closed this Aug 3, 2016
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.

4 participants