-
Notifications
You must be signed in to change notification settings - Fork 598
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[GitLab] support for using forks #1293
Comments
changes for webhook handling new commits from a fork added after pr-agent/pr_agent/servers/gitlab_webhook.py Lines 188 to 197 in e82afdd
elif data.get('object_kind') == 'merge_request' and data['object_attributes'].get('action') == 'update':
try:
title = data['object_attributes'].get('title')
project_id = data['object_attributes'].get('target_project_id')
commit_sha = data['object_attributes']['last_commit'].get('id')
url = await get_mr_url_from_commit_sha(commit_sha, gitlab_token, project_id)
draft = data['object_attributes'].get('draft')
get_logger().info(f"A merge request has been updated: {url}")
if draft:
get_logger().info(f"Skipping draft MR: {url}")
return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"}))
if not url:
get_logger().info(f"No MR found for commit: {commit_sha}")
return JSONResponse(status_code=status.HTTP_200_OK,
content=jsonable_encoder({"message": "success"}))
# we need first to apply_repo_settings
apply_repo_settings(url)
commands_on_push = get_settings().get(f"gitlab.push_commands", {})
handle_push_trigger = get_settings().get(f"gitlab.handle_push_trigger", False)
if not commands_on_push or not handle_push_trigger:
get_logger().info("Push event, but no push commands found or push trigger is disabled")
return JSONResponse(status_code=status.HTTP_200_OK,
content=jsonable_encoder({"message": "success"}))
await _perform_commands_gitlab("push_commands", PRAgent(), url, log_context, data)
except Exception as e:
get_logger().error(f"Failed to handle updated MR: {e}") |
Hi i am not sure i fully understand the issue, or the proposed solution. Basically, after a fork the desired behavior is not to invoke the pr-agent, and discard the webhook 'gracefully' For GitHub, we have some check for that:
A similar solution might be relevant for GitLab (although forks there are less frequent, as there are not a lot of public GitLab repos) calling If there is an easier way to understand if the webhook came from a GitLab fork, it can be relevant for merge to the main branch |
sorry this is a PR that originates from a fork.. so the workflow should not run in the fork but it should run on an open PR (which has had new commits pushed after the PR is first opened) that is opened from a fork to the main project? as for the URL i was not sure if it was tied into the links for the PR but it seems you can get the url from the webhook without having to run this. the url is under the
|
I searched open issues and couldn’t find anything related and I also didn’t see anything in the docs that calls out using branches over forks so I apologize in advance if this is working as intended or I missed anything.
We have been using the agent as a webhook app in our installation and this has been working great! However I have noticed a few small things related to the use of forks vs branches.
when using a fork the agent will not be triggered by the commit webhook as the commit was to the fork not the main project. GitLab however does send a webhook when a open merge request has a new commit as a merge request webhook in my local install I made a minor change to trigger the commit workflow off this webhook and it works perfectly. Example to follow.
the second issue I observed is with the code links in the generated describe and review and improve comments all the links use a url which is generated to work from a pr that originates from a branch but when it comes from a fork the url format changes it would need to be the hash of the commit and file instead of the branch name.
I am writing from mobile currently but I will follow up with examples/specifics.
The text was updated successfully, but these errors were encountered: