search

iterative / cml

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

pr + comment create Error: PR for commit sha not found #1418

Open BradyJ27 opened 1 year ago

BradyJ27 commented 1 year ago

I have a basic GitLab CI pipeline that looks like: Related to https://discord.com/channels/485586884165107732/728693131557732403/1144379101176660029

cml pr .
echo README.md >> report.md
cml comment create --target=pr report.md

I am getting an error: 

{"level":"error","message":"PR for commit sha \"4d2069245c68a65774a4a0d64a1efb3611a93b55\" not found","stack":"Error: PR for commit sha \"4d2069245c68a65774a4a0d64a1efb3611a93b55\" not found\n    at parseCommentTarget (/usr/lib/node_modules/@dvcorg/cml/src/commenttarget.js:54:13)\n    at processTicksAndRejections (node:internal/process/task_queues:96:5)\n    at async CML.commentCreate (/usr/lib/node_modules/@dvcorg/cml/src/cml.js:308:20)\n    at async Object.exports.handler (/usr/lib/node_modules/@dvcorg/cml/bin/cml/comment/create.js:11:15)"}

I've set up an example repo with the run results here: https://gitlab.com/bjohnson210/cml-pr-bug/-/jobs/4945761331 to reproduce this issue.

dacbd commented 1 year ago

In your example, it looks like a PR was never created, so it couldn't find PR with that commit in it.

In your original discord message maybe there was a bit of a race condition in gitlab hadn't yet created the PR when cml comment create attempted to send the API request to gitlab.

BradyJ27 commented 1 year ago

In your example, it looks like a PR was never created, so it couldn't find PR with that commit in it.

In your original discord message maybe there was a bit of a race condition in gitlab hadn't yet created the PR when cml comment create attempted to send the API request to gitlab.

Oops, the Public GitLab runners are on 16.x which causes some issue which was fixed in #1404 , but that hasn't been part of a release yet. I don't know if GitLab has any runners on older versions where I can reproduce. I will test to make sure it's not a race condition

dacbd commented 1 year ago

Ahh yes sorry, we had an issue with our release process, I'll take another look tomorrow and see if we can get a proper release done.

On Thu, Aug 24, 2023, 17:31 BradyJ27 @.***> wrote:

Oops, the Public GitLab runners are on 16.x which causes some issue which was fixed in #1404 https://github.com/iterative/cml/pull/1404 , but that hasn't been part of a release yet. I don't know if GitLab has any runners on older versions where I can reproduce. I will test to make sure it's not a race condition

— Reply to this email directly, view it on GitHub https://github.com/iterative/cml/issues/1418#issuecomment-1692586802, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIN7M5FOKM27R7DRXVH4QLXW7W47ANCNFSM6AAAAAA35WBJKU . You are receiving this because you commented.Message ID: @.***>

BradyJ27 commented 1 year ago

After adding a sleep 30 (in my private repo) I am still getting the same error. I am using cml pr . instead of cml pr create ., not sure if that makes a difference.

dacbd commented 1 year ago

@BradyJ27 I've completed a new release. Can you check and see if this solved the problem?

BradyJ27 commented 1 year ago

@dacbd It looks like GitLab CI isn't able to pull the newest version of the docker image, it is still running on CML v0.19.1. It may take some time for it to update, so I can try again in a little bit. Let me know if you see anything wrong with the setup below

Here is my .gitlab-ci.yml:

train-and-report:
  image: iterativeai/cml:latest
  script:
    - cml --version
    - cml ci
    - echo "Add new file" >> file.txt
    - cml pr .
    - cat README.md >> report.md
    - sleep 30
    - cml comment create --target=pr report.md

and here is the output of the job:

image

dacbd commented 1 year ago

@BradyJ27 sorry, I was able to rebuild the images, give it another try for me

BradyJ27 commented 1 year ago

@dacbd Yep, I was able to reproduce now with updated image. https://gitlab.com/bjohnson210/cml-pr-bug/-/jobs/4956922294

cml pr . outputs https://gitlab.com/bjohnson210/cml-pr-bug/-/merge_requests/1 then cml comment create --target=pr outputs: 

{"level":"error","message":"PR for commit sha \"78350418cc66e1d6e931fcf3d1eea99190ad01d7\" not found","stack":"Error: PR for commit sha \"78350418cc66e1d6e931fcf3d1eea99190ad01d7\" not found\n    at parseCommentTarget (/usr/lib/node_modules/@dvcorg/cml/src/commenttarget.js:54:13)\n    at processTicksAndRejections (node:internal/process/task_queues:96:5)\n    at async CML.commentCreate (/usr/lib/node_modules/@dvcorg/cml/src/cml.js:308:20)\n    at async Object.exports.handler (/usr/lib/node_modules/@dvcorg/cml/bin/cml/comment/create.js:11:15)"}

I'm assuming it is not a race condition because there is a 30 second sleep between the two.

dacbd commented 1 year ago

Ok then I'll have look deeper and try to find out what the issue is.

On Fri, Aug 25, 2023, 12:33 BradyJ27 @.***> wrote:

@dacbd https://github.com/dacbd Yep, I was able to reproduce now with updated image. https://gitlab.com/bjohnson210/cml-pr-bug/-/jobs/4956922294

cml pr . outputs https://gitlab.com/bjohnson210/cml-pr-bug/-/merge_requests/1 then cml comment create --target=pr outputs: {"level":"error","message":"PR for commit sha \"78350418cc66e1d6e931fcf3d1eea99190ad01d7\" not found","stack":"Error: PR for commit sha \"78350418cc66e1d6e931fcf3d1eea99190ad01d7\" not found\n at parseCommentTarget @./cml/src/commenttarget.js:54:13)\n at processTicksAndRejections (node:internal/process/task_queues:96:5)\n at async CML.commentCreate @./cml/src/cml.js:308:20)\n at async Object.exports.handler @.***/cml/bin/cml/comment/create.js:11:15)"}

I'm assuming it is not a race condition because there is a 30 second sleep between the two.

— Reply to this email directly, view it on GitHub https://github.com/iterative/cml/issues/1418#issuecomment-1693838212, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIN7MZ6CWD5BTXRGOKZLYTXXD4XPANCNFSM6AAAAAA35WBJKU . You are receiving this because you were mentioned.Message ID: @.***>

BradyJ27 commented 1 year ago

Hey @dacbd, have you had a chance to look into this at all?

dacbd commented 1 year ago

@BradyJ27 no sorry I havent been able to get to this, as a work around I would add report.md to you .gitignore and then you can set the body of pr to your report contents like so cml pr --body="$(cat ./report.md)" . you can see here: https://cml.dev/doc/ref/pr#--body

BradyJ27 commented 1 year ago

@BradyJ27 no sorry I havent been able to get to this, as a work around I would add report.md to you .gitignore and then you can set the body of pr to your report contents like so cml pr --body="$(cat ./report.md)" . you can see here: https://cml.dev/doc/ref/pr#--body

Sure, this will work, thanks!

BradyJ27 commented 5 months ago

As info, this issue still persists. If there are any ideas on what needs to change I'd be willing to contribute here.

dacbd commented 5 months ago

@BradyJ27 sorry, I don't think any of the contributors have the capacity right now to debug and solve this.

You are more than welcome to patch and contribute changes. There are a few places that you will likely need to edit.

I would work backwards from the gitlab driver