Skip to content

Support Github auto-format for retriggering with other PR #2923

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.

Sign up for GitHub

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

Merged
centosinfra-prod-github-app merged 1 commit into from
Jan 5, 2026
Merged

Support Github auto-format for retriggering with other PR #2923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions packit_service/worker/helpers/testing_farm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,22 @@ def parse_known_arguments(self, args: argparse.Namespace) -> None:

def parse_unknown_arguments(self, unknown_args: list[str]) -> None:
# Process unknown_args to find pr_argument
# Supports these formats:
# - namespace/repo#<pr_id>
# - https://github.com/namespace/repo/pull/<pr_id> (GitHub auto-converts the above to this)
pr_argument_pattern = re.compile(r"^[^/\s]+/[^#\s]+#\d+$")
github_url_pattern = re.compile(r"^https://github\.com/([^/\s]+)/([^/\s]+)/pull/(\d+)$")
Comment on lines 140 to +141
Copy link
Copy Markdown
Member

@mfocko mfocko Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably wouldn't be a bad idea to have one regex for both, since you can capture what you need from both;

Copy link
Copy Markdown
Member

@mfocko mfocko Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally it wouldn't be that bad of an idea to compile the regex(es) only once, e.g., in constants?

Copy link
Copy Markdown
Member

@mfocko mfocko Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://regex101.com/r/sX7EJU/1

Note

  1. I dropped the ^ and $ for boundaries, since it didn't work well with whole comments I copy-pasted
  2. Also used named capture groups, since the alternatives must also be enclosed in parentheses and therefore make a capture group, haven't checked the Python docs, but I ticked the Python syntax on that web, so it should be allowed.

Copy link
Copy Markdown
Member Author

@lbarcziova lbarcziova Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the two separate patterns keep the validation stricter (e.g. multi-level namespaces) and more explicit, so I would probably keep it that way

for arg in unknown_args:
if pr_argument_pattern.match(arg):
self.pr_argument = arg
logger.debug(f"Parsed pr_argument: {self.pr_argument}")
break
if match := github_url_pattern.match(arg):
Copy link
Copy Markdown
Member

@mfocko mfocko Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We support this only for GitHub?

Or GitHub is the only “UX-friendly” enough to do this crap?

am afraid I know the answers to those two questions sigh

Copy link
Copy Markdown
Member Author

@lbarcziova lbarcziova Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Gitlab actually doesn't use the #<pr_id>, but !<pr_id>, so there shouldn't be any "conversion" happenning

# Convert GitHub URL format back to namespace/repo#pr_id format
namespace, repo, pr_id = match.groups()
self.pr_argument = f"{namespace}/{repo}#{pr_id}"
logger.debug(f"Parsed pr_argument from GitHub URL: {arg} -> {self.pr_argument}")
break


class TestingFarmJobHelper(CoprBuildJobHelper):
Expand Down
22 changes: 22 additions & 0 deletions tests/unit/test_testing_farm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2288,6 +2288,28 @@ def test_is_supported_architecture(target, use_internal_tf, supported):
"namespace-2/repo-2#36",
{"IP_FAMILY": "ipv6", "INSTALL_TYPE": ""},
),
# Test GitHub URL format (auto-converted by GitHub)
(
"/packit-dev test https://github.com/kontura/librepo/pull/4",
None,
None,
"kontura/librepo#4",
None,
),
(
"/packit-dev test https://github.com/namespace-3/repo-3/pull/42 --identifier my-id-3",
"my-id-3",
None,
"namespace-3/repo-3#42",
None,
),
(
"/packit-dev test --labels label1,label2 https://github.com/namespace-4/repo-4/pull/99",
None,
["label1", "label2"],
"namespace-4/repo-4#99",
None,
),
],
)
def test_parse_comment_arguments(
Expand Down
Loading