Feature/pipeline schedules - #398
Conversation
| obj_cls = ProjectFile | ||
|
|
||
|
|
||
| class ProjectPipelineSchedule(GitlabObject): |
There was a problem hiding this comment.
I don't think pipelines schedule are implemented in the v3 API, so this file should probably not be changed.
| _obj_cls = ProjectPipelineScheduleVariable | ||
| _from_parent_attrs = {'project_id': 'project_id', | ||
| 'pipeline_schedule_id' : 'id'} | ||
| _create_attrs = (('pipeline_schedule_id', 'key', 'value'), tuple()) |
There was a problem hiding this comment.
You can remove this line since you redefine _create_attrs on the next line.
| _create_attrs = (('pipeline_schedule_id', 'key', 'value'), tuple()) | ||
| _create_attrs = (('key', 'value'), tuple()) | ||
|
|
||
| def list(self): |
There was a problem hiding this comment.
I'm not sure it's a good idea to implement this method. I'd like to avoid adding features that are not directly coming from the GitLab API. I'd rather remove this method.
| ('notificationsettings', 'ProjectNotificationSettingsManager'), | ||
| ('pipelines', 'ProjectPipelineManager'), | ||
| ('protectedbranches', 'ProjectProtectedBranchManager'), | ||
| ('pipeline_schedules', 'ProjectPipelineSchedulesManager'), |
There was a problem hiding this comment.
Could you use pipelineschedules for consistency with the rest of the attribute names?
| return array | ||
|
|
||
|
|
||
| class ProjectPipelineSchedule(RESTObject): |
There was a problem hiding this comment.
GitLab supports updating and deleting these resources, so you need to add the SaveMixin and ObjectDeleteMixin here.
| ) | ||
|
|
||
|
|
||
| class ProjectPipelineSchedulesManager(RetrieveMixin, CreateMixin, RESTManager): |
There was a problem hiding this comment.
You can use the CRUDMixin since all the methods are supported.
There was a problem hiding this comment.
And the class should be called ProjectPipelineScheduleManager (no plural).
| _from_parent_attrs = {'project_id': 'id'} | ||
| _create_attrs = (('description', 'ref', 'cron'), | ||
| ('cron_timezone', 'active')) | ||
|
|
There was a problem hiding this comment.
Could you define the _update_attrs attributes as well?
| _create_attrs = (('description', 'ref', 'cron'), | ||
| ('cron_timezone', 'active')) | ||
|
|
||
| def create(self, data, **kwargs): |
There was a problem hiding this comment.
I don't think you need this method (you're just calling the mixin method).
| return utils.response_content(result, streamed, action, chunk_size) | ||
|
|
||
|
|
||
| class ProjectPipelineJob(ProjectJob): |
There was a problem hiding this comment.
Pipeline jobs support has already be implemented, could you remove these items?
|
@mlq if you don't mind I'll update your patch to fix the problems listed in the review. |
|
Hi! Sorry that I haven't replied yet but I haven't found the time to introduce your suggested changes. If you can do that, it would be great! I just tried to build something that was working for me without knowing everything about the implementation! Thank you! |
|
Thanks for your fast answer! I'll update your patch. |
|
Thanks for the initial work @mlq! |
|
Awesome, thank you for this project! |
Hi,
I've started to implement support for pipeline schedules and wanted to ask if this is wanted to be picked up and reviewed. It allows to add pipeline schedules as well as variables. Any feedback is welcome as I am not so familiar with the code base.
Best regards