search

iterative / cml

♾️ CML - Continuous Machine Learning | CI/CD for ML
http://cml.dev
Apache License 2.0
4.01k stars 341 forks source link

CML comment create/update image links not working anymore in Gitlab #1478

Open jancrichter opened 1 week ago

jancrichter commented 1 week ago

Hi everyone,

We are using Gitlab 17.4.1 and CML 0.20.4. We have recently started noticing that images added to MR comments via cml comment create or cml comment update, while uploaded correctly, cannot be accessed under the link CML puts into the MR comment.

CML puts in absolute URLs like

![](https://${myinstance}.gitlab.com/${mygroup}/${mysubgroup}/${myproject}/uploads/6aaacfcf584e185f78b6df984148ad6a/plot.png?cml=png&cache-bypass=b4c2a1c9-0d6b-41b4-975b-168e1d150e12 "plot_title")

For new file uploads in Gitlab 17 these URLs do not work anymore (they continue working for uploads that already exist).

What works instead is this:

![](https://${myinstance}.gitlab.com/-/projects/${myprojectsid}/uploads/6aaacfcf584e185f78b6df984148ad6a/plot.png?cml=png&cache-bypass=b4c2a1c9-0d6b-41b4-975b-168e1d150e12 "plot_title")

I was not able to find a change in Gitlab 17 that would explain this behaviour yet, so it might be configuration on our end. I talked to the admins but they aren't aware of anything either.

I would like to propose switching to a more robust version of the links though. When users manually upload files to a comment, Gitlab simply puts in a relative path like this:

![](/uploads/6aaacfcf584e185f78b6df984148ad6a/plot.png?cml=png&cache-bypass=b4c2a1c9-0d6b-41b4-975b-168e1d150e12 "plot_title")

Is there any reason not to use this approach?

Update: Forgot to mention this above, but from what I saw from the CML code the currently used links are put together from the Gitlab API response of the projects/uploads endpoint and the project path. The former part being exactly that relative URL/path mentioned in my last code snippet. So, seemingly, all that would need to be done is to change

https://github.com/iterative/cml/blob/5e9fcd26e9683db26a429eb5f34b989134694d4b/src/drivers/gitlab.js#L144

and drop ${repo} from it. In my rather naive attempt of testing that it led to an assertion error for the URL though.

shcheklein commented 1 week ago

Is there any reason not to use this approach?

Seems reasonable. @skshetry do you have any information on this?