job.refresh() doesn't exist but is needed (hacky fix included)

[1ace]

Description of the problem, including code/CLI snippet

When finding a manual job in a pipeline and deciding to .play() it (through project.jobs.get(pipeline_job.id).play() since .play() is not supported on pipeline jobs), the pipeline job status doesn't get updated.

One would think we might need to call pipelin_job.refresh() to get the updated status, but this crashes instead, as the method doesn't exist on jobs.

Expected Behavior

job.refresh() refreshes the job data, just like pipeline.refresh() does.

Actual Behavior

  File ".../gitlab/base.py", line 134, in __getattr__
    raise AttributeError(message) from exc
AttributeError: 'ProjectPipelineJob' object has no attribute 'refresh'

Specifications

• python-gitlab version: 4.4.0
• API version you are using (v3/v4): v4 (but not relevant for this issue)
• Gitlab server version (or gitlab.com): 16.8.1 (but not relevant for this issue)

---

Looks like ProjectPipelineJob is missing the RefreshMixin, but just adding it doesn't work; refresh() does a self.manager.gitlab.http_get() with an incorrect path, and gets a 404 in return.
The path used is /projects/{project_id}/pipelines/{pipeline_id}/jobs/{job_id} but should be /projects/{project_id}/jobs/{job_id}.

The following crude patch fixes this issue, but someone who understand the python-gitlab codebase better than me should be able to fix this cleanly instead 🙂

diff --git a/gitlab/mixins.py b/gitlab/mixins.py
index a232c31e31d88e0eadbe..9b0eb414a879aef99c46 100644
--- a/gitlab/mixins.py
+++ b/gitlab/mixins.py
@@ -186,6 +186,9 @@ def refresh(self, **kwargs: Any) -> None:
             if TYPE_CHECKING:
                 assert self.manager.path is not None
             path = self.manager.path
+        if '/pipelines/' in path and '/jobs/' in path:
+            import re
+            path = re.sub(r'/pipelines/\d+', '', path)
         server_data = self.manager.gitlab.http_get(path, **kwargs)
         if TYPE_CHECKING:
             assert not isinstance(server_data, requests.Response)
diff --git a/gitlab/v4/objects/pipelines.py b/gitlab/v4/objects/pipelines.py
index 75306119b255c471611c..51ad470791ab3c032132 100644
--- a/gitlab/v4/objects/pipelines.py
+++ b/gitlab/v4/objects/pipelines.py
@@ -140,7 +140,7 @@ def create(
         )
 
 
-class ProjectPipelineJob(RESTObject):
+class ProjectPipelineJob(RefreshMixin, RESTObject):
     pass
 
 

[1ace]

While trying to understand the code to find a better fix for the above, I noticed that ProjectJob's .play() also doesn't update the job status; #2784 fixes that

[1ace]

I don't understand why ProjectPipelineJob exists though; why doesn't ProjectPipelineJobManager.list() return a list of ProjectJob?
That would resolve a bunch of limitations, and I don't see the downside/blocker for doing that, so I must be missing something 😅

[nejch]

@1ace they're different objects because they're created from different endpoints, and don't necessarily have the same amount of details returned (or even the same attribute structure, as we've found out recently with merge request reviewer details returned from different endpoints). So it could be a bit confusing changing the object attributes dynamically like that. And the GitLab API is too inconsistent for us to assume they could be considered the same type of object IMO.

We sort of follow the REST API path segment structure for the most part with this approach. We also document this a bit:

This is far from the only case, so if we wanted to be consistent here, we would have to add this path hack to almost every endpoint where similar-but-not-quite-the-same resources are returned. This is why for now we simply document how to get an editable resource when fetched from a different endpoint:

https://python-gitlab.readthedocs.io/en/stable/faq.html#i-cannot-edit-the-merge-request-issue-i-ve-just-retrieved

However, if refresh() is needed for ProjectJob we should be able to add that. But technically there is no single GET call for /pipelines/:id/jobs/:job_id and this is why we don't have a refresh mixin for that one.

[nejch]

@1ace with #2784 merged, does this cover your use case along with the docs above?