Skip to content
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

Jump to bottom

Add --env to comment trigger phrase #2583

Merged
merged 1 commit into from
Oct 15, 2024
Merged

Add --env to comment trigger phrase #2583

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
112 changes: 88 additions & 24 deletions packit_service/worker/helpers/testing_farm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
# Copyright Contributors to the Packit project.
# SPDX-License-Identifier: MIT

import argparse
import logging
import re
from re import Pattern
import shlex
from typing import Dict, Any, Optional, Set, List, Union, Tuple, Callable

import requests
Expand Down Expand Up @@ -60,40 +61,92 @@ class CommentArguments:
Parse arguments from trigger comment and provide the attributes to Testing Farm helper.
"""

packit_command: str = None
identifier: str = None
labels: List[str] = None
pr_argument: str = None

def __init__(self, command_prefix: str, comment: str):
Copy link
Member

Choose a reason for hiding this comment

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

the __init__ method grew quite a lot with the new code, do you think it could be split up a bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will try to split it

self._parser: argparse.ArgumentParser = None
self.packit_command: str = None
self.identifier: str = None
self.labels: List[str] = None
self.pr_argument: str = None
self.envs: Dict[str, str] = None

if comment is None:
return

# Try to parse identifier argument from comment
# Remove the command prefix from the comment
logger.debug(f"Parsing comment -> {comment}")
logger.debug(f"Used command prefix -> {command_prefix}")

match = re.search(
r"^" + re.escape(command_prefix) + r"\s(?P<packit_command>\S+)", comment
)
# Match the command prefix and extract the rest of the comment
match = re.match(r"^" + re.escape(command_prefix) + r"\s+(.*)", comment)
if match:
self.packit_command = match.group("packit_command")
logger.debug(f"Parsed packit_command: {self.packit_command}")
arguments_str = match.group(1)
else:
# If the command prefix is not found, nothing to parse
logger.debug("Command prefix not found in the comment.")
return

match = re.search(r"(--identifier|--id|-i)[\s=](?P<identifier>\S+)", comment)
if match:
self.identifier = match.group("identifier")
logger.debug(f"Parsed test argument -> identifier: {self.identifier}")
# Use shlex to split the arguments string into a list
args_list = shlex.split(arguments_str)
logger.debug(f"Arguments list after shlex splitting: {args_list}")

match = re.search(r"--labels[\s=](?P<labels>\S+)", comment)
if match:
self.labels = match.group("labels").split(",")
logger.debug(f"Parsed test argument -> labels: {self.labels}")
# Parse known arguments
try:
args, unknown_args = self.parser.parse_known_args(args_list)
logger.debug(f"Parsed known args: {args}")
logger.debug(f"Unknown args: {unknown_args}")
except argparse.ArgumentError as e:
logger.error(f"Argument parsing error: {e}")
return

match = re.search(r"(?P<pr_arg>[^/\s]+/[^#]+#\d+)", comment)
if match:
self.pr_argument = match.group("pr_arg")
logger.debug(f"Parsed test argument -> pr_argument: {self.pr_argument}")
self.parse_known_arguments(args)
self.parse_unknown_arguments(unknown_args)

@property
def parser(self) -> argparse.ArgumentParser:
if self._parser is None:
# Set up argparse
self._parser = argparse.ArgumentParser()
self._parser.add_argument("packit_command")
self._parser.add_argument("--identifier", "--id", "-i")
self._parser.add_argument("--labels", type=lambda s: s.split(","))
# Allows multiple --env arguments
self._parser.add_argument("--env", action="append")

return self._parser

def parse_known_arguments(self, args: argparse.Namespace) -> None:
# Assign the parsed arguments to the class attributes
self.packit_command = args.packit_command
logger.debug(f"Parsed packit_command: {self.packit_command}")

self.identifier = args.identifier
logger.debug(f"Parsed identifier: {self.identifier}")

if args.labels:
self.labels = args.labels
logger.debug(f"Parsed labels: {self.labels}")

if args.env:
self.envs = {}
for env in args.env:
if "=" in env:
key, value = env.split("=", 1)
self.envs[key] = value
logger.debug(f"Parsed env variable: {key}={value}")
else:
logger.error(
f"Invalid format for '--env' argument: '{env}'. Expected VAR_NAME=value."
)
continue

def parse_unknown_arguments(self, unknown_args: List[str]) -> None:
# Process unknown_args to find pr_argument
pr_argument_pattern = re.compile(r"^[^/\s]+/[^#\s]+#\d+$")
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


class TestingFarmJobHelper(CoprBuildJobHelper):
Expand Down Expand Up @@ -523,6 +576,17 @@ def _payload(
env_variables = self.job_config.env if hasattr(self.job_config, "env") else {}
predefined_environment.update(env_variables)

# User-defined variables from comments have priority
if self.is_comment_event and self.comment_arguments.envs is not None:
for k, v in self.comment_arguments.envs.items():
# Set env variable
logger.debug(f"Key: {k} -> Value: '{v}'")
if v is not None and v != "":
predefined_environment[k] = v
# Unset env variable if it doesn't have value
else:
predefined_environment.pop(k, None)

environment: Dict[str, Any] = {
"arch": arch,
"os": {"compose": compose},
Expand Down
40 changes: 20 additions & 20 deletions tests/unit/test_checkers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,27 +507,27 @@ def test_koji_branch_merge_queue():
"comment, result",
(
pytest.param(
"/packit-dev test --identifier my-id-1",
"/packit test --identifier my-id-1",
True,
id="Matching identifier specified",
),
pytest.param(
"/packit-dev test --id my-id-1",
"/packit test --id my-id-1",
True,
id="Matching identifier specified",
),
pytest.param(
"/packit-dev test -i my-id-1",
"/packit test -i my-id-1",
True,
id="Matching identifier specified",
),
pytest.param(
"/packit-dev test",
"/packit test",
True,
id="No identifier specified",
),
pytest.param(
"/packit-dev test --identifier my-id-2",
"/packit test --identifier my-id-2",
False,
id="Non-matching identifier specified",
),
Expand Down Expand Up @@ -582,35 +582,35 @@ def test_tf_comment_identifier(comment, result):
"comment, default_identifier, job_identifier, result",
(
pytest.param(
"/packit-dev test --identifier my-id2",
"/packit test --identifier my-id2",
"id1",
"id1",
False,
id="Identifier specified in comment",
),
pytest.param(
"/packit-dev test",
"/packit test",
None,
"id1",
True,
id="No identifier specified, no default identifier",
),
pytest.param(
"/packit-dev test",
"/packit test",
"id1",
"id1",
True,
id="No identifier specified, default identifier matching",
),
pytest.param(
"/packit-dev test",
"/packit test",
"id1",
"id2",
False,
id="No identifier specified, default identifier not matching",
),
pytest.param(
"/packit-dev test",
"/packit test",
"id1",
None,
False,
Expand Down Expand Up @@ -670,17 +670,17 @@ def test_tf_comment_default_identifier(
"comment, result",
(
pytest.param(
"/packit-dev test --labels label1,label2",
"/packit test --labels label1,label2",
True,
id="Matching label specified",
),
pytest.param(
"/packit-dev test",
"/packit test",
True,
id="No labels specified",
),
pytest.param(
"/packit-dev test --labels random-label1,random-label2",
"/packit test --labels random-label1,random-label2",
False,
id="Non-matching label specified",
),
Expand Down Expand Up @@ -736,35 +736,35 @@ def test_tf_comment_labels(comment, result):
"comment, default_labels, job_labels, result",
(
pytest.param(
"/packit-dev test --labels label1,label2",
"/packit test --labels label1,label2",
["label3"],
["label3"],
False,
id="Labels specified in comment",
),
pytest.param(
"/packit-dev test",
"/packit test",
None,
["label1"],
True,
id="No labels specified, no default labels",
),
pytest.param(
"/packit-dev test",
"/packit test",
["label2"],
["label1", "label2"],
True,
id="No labels specified, default labels matching",
),
pytest.param(
"/packit-dev test",
"/packit test",
["label3"],
["label1", "label2"],
False,
id="No labels specified, default labels not matching",
),
pytest.param(
"/packit-dev test",
"/packit test",
["label3"],
[],
False,
Expand Down Expand Up @@ -829,12 +829,12 @@ def test_tf_comment_default_labels(comment, default_labels, job_labels, result):
"comment, result",
(
pytest.param(
"/packit-dev test",
"/packit test",
True,
id="No labels specified, none in config: should pass",
),
pytest.param(
"/packit-dev test --labels should_fail,should_fail_hard",
"/packit test --labels should_fail,should_fail_hard",
False,
id="Labels specified, none in config: should fail",
),
Expand Down
Loading
Loading