- Notifications
You must be signed in to change notification settings - Fork673
Add manager for jobs within a pipeline.#413
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for this change! Could you have a look at my comments and make the suggested modifications before I merge?
If could could add a couple examples (list and get) in the documentation that would be very nice.
Thanks again 👍
gitlab/v4/objects.py Outdated
| @@ -1940,6 +1942,12 @@ def create(self, data, **kwargs): | |||
| return CreateMixin.create(self, data, path=path, **kwargs) | |||
| class ProjectPipelineJobManager(RetrieveMixin, RESTManager): | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Could you useGetWithoutIdMixin instead ofRetrieveMixin?
It looks like getting a single job is not possible with the GitLab API, but we can simulate this feature by looping through the list.
gitlab/v4/objects.py Outdated
| @@ -1940,6 +1942,12 @@ def create(self, data, **kwargs): | |||
| return CreateMixin.create(self, data, path=path, **kwargs) | |||
| class ProjectPipelineJobManager(RetrieveMixin, RESTManager): | |||
| _path = '/projects/%(project_id)s/pipelines/%(pipeline_id)s/jobs' | |||
| _obj_cls = ProjectJob | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'd rather use a newProjectPipelineJob instead ofProjectJob. This is choice I made for other objects and managers, so I'd rather stick to this logic.
Review markup: better to use a distinct ProjectPipelineJob ratherthan sharing ProjectJob. This is consistent with the other objectsand managers.
Thanks@gpocentek - I've added some docs, distinguished the classes as you requested (I just made an In principle we could implement Thanks. |
kw217 commentedJan 15, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Not sure why the Python 2.7 build failed, but it looks like a connection error pulling one of the libraries, not related to my change :-( Edit: it seems to have passed now. |
@kw217 Thanks! |
Great, thank you! |
Hi@gpocentek - when are you likely to spin a new release containing this enhancement? I'd like to use it in a tool I'm writing, and it will be simpler to point people at PyPI than getting them to download master. Thanks. |
Super - that would be great. Thanks! |
This PR adds a manager for jobs within a pipeline, so you can say
gl.projects.get(prid).pipelines.get(plid).jobs.list()to list all of the jobs within a single pipeline (in GitLab v4).Please let me know if there are any tests I should add. The existing UTs still pass.