From 3635fc6dce6f8a3e91175dffb3029a5b5ebf7646 Mon Sep 17 00:00:00 2001 From: Tomas Hrcka Date: May 29 2023 12:26:59 +0000 Subject: [PATCH 1/6] Add python 3.11 into zuul config Signed-off-by: Tomas Hrcka --- diff --git a/.zuul.yaml b/.zuul.yaml index 9029977..0d60eff 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -15,30 +15,35 @@ - fi-tox-lint: vars: tox_envlist: flake8 - - fi-tox-python38: + - fi-tox-python39: vars: dependencies: - cairo-devel - cairo-gobject-devel - gobject-introspection-devel - libmodulemd1 - - fi-tox-python39: + - fi-tox-python310: vars: dependencies: - cairo-devel - cairo-gobject-devel - gobject-introspection-devel - libmodulemd1 - - fi-tox-python310: + # Following dependencies are needed because tests in zuul CI + # can't see them inside virtualenv + - python3-pluggy + - python3-py + - python3-toml + - fi-tox-python311: vars: dependencies: - cairo-devel - cairo-gobject-devel - gobject-introspection-devel - - libmodulemd1 + - libmodulemd2 # Following dependencies are needed because tests in zuul CI # can't see them inside virtualenv - python3-pluggy - python3-py - python3-toml -... +... \ No newline at end of file From 4d53cbb71db63e4e32d60f44b08971d99291ebdc Mon Sep 17 00:00:00 2001 From: Tomas Hrcka Date: May 30 2023 06:46:29 +0000 Subject: [PATCH 2/6] Update libmodulemd to v2 Signed-off-by: Tomas Hrcka --- diff --git a/.zuul.yaml b/.zuul.yaml index 0d60eff..b457abb 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -8,7 +8,7 @@ - cairo-devel - cairo-gobject-devel - gobject-introspection-devel - - libmodulemd1 + - libmodulemd - fi-tox-format: vars: tox_envlist: black @@ -21,14 +21,14 @@ - cairo-devel - cairo-gobject-devel - gobject-introspection-devel - - libmodulemd1 + - libmodulemd - fi-tox-python310: vars: dependencies: - cairo-devel - cairo-gobject-devel - gobject-introspection-devel - - libmodulemd1 + - libmodulemd # Following dependencies are needed because tests in zuul CI # can't see them inside virtualenv - python3-pluggy @@ -40,7 +40,7 @@ - cairo-devel - cairo-gobject-devel - gobject-introspection-devel - - libmodulemd2 + - libmodulemd # Following dependencies are needed because tests in zuul CI # can't see them inside virtualenv - python3-pluggy diff --git a/Dockerfile b/Dockerfile index 0f1dbbf..b8f2647 100644 --- a/Dockerfile +++ b/Dockerfile @@ -8,7 +8,7 @@ RUN dnf -y install \ cairo-gobject-devel \ fedora-messaging \ gobject-introspection-devel \ - libmodulemd1 \ + libmodulemd \ python3-arrow \ python3-beanbag \ python3-beautifulsoup4 \ diff --git a/toddlers/plugins/pdc_modules.py b/toddlers/plugins/pdc_modules.py index 390c97a..1d0cf68 100644 --- a/toddlers/plugins/pdc_modules.py +++ b/toddlers/plugins/pdc_modules.py @@ -19,7 +19,7 @@ from ..base import ToddlerBase from ..utils.pdc import pdc_client_for_config from ..utils.requests import make_session -gi.require_version("Modulemd", "1.0") +gi.require_version("Modulemd", "2.0") from gi.repository import Modulemd # noqa: E402, I100 _log = logging.getLogger(__name__) diff --git a/toddlers/utils/requests.py b/toddlers/utils/requests.py index 251e3fb..63b7ed0 100644 --- a/toddlers/utils/requests.py +++ b/toddlers/utils/requests.py @@ -5,7 +5,7 @@ from typing import Any, Optional, Tuple, Union import requests from requests.packages.urllib3.util import retry -from requests.packages.urllib3.util.timeout import _Default # type: ignore +from requests.packages.urllib3.util.timeout import _DEFAULT_TIMEOUT as _Default # type: ignore # mypy couldn't find the typing information for default diff --git a/tox.ini b/tox.ini index d36a6c7..0a7a345 100644 --- a/tox.ini +++ b/tox.ini @@ -22,7 +22,7 @@ commands = black --check --diff . [testenv:mypy] -basepython = python3.11 +basepython = python3.10 deps = {[testenv]deps} mypy From 8a7f65aea7bf08c020deb5521ba9edaf78cd26f1 Mon Sep 17 00:00:00 2001 From: Tomas Hrcka Date: May 30 2023 07:09:28 +0000 Subject: [PATCH 3/6] update requests and tox Signed-off-by: Tomas Hrcka --- diff --git a/toddlers/utils/requests.py b/toddlers/utils/requests.py index 63b7ed0..521722a 100644 --- a/toddlers/utils/requests.py +++ b/toddlers/utils/requests.py @@ -4,12 +4,12 @@ from typing import Any, Optional, Tuple, Union import requests -from requests.packages.urllib3.util import retry -from requests.packages.urllib3.util.timeout import _DEFAULT_TIMEOUT as _Default # type: ignore +from urllib3.util import retry +from urllib3.util.timeout import Timeout as timeout# type: ignore # mypy couldn't find the typing information for default - +_Default = timeout.DEFAULT_TIMEOUT __all__ = ["make_session"] RETRY_DEFAULTS = { diff --git a/tox.ini b/tox.ini index 0a7a345..d36a6c7 100644 --- a/tox.ini +++ b/tox.ini @@ -22,7 +22,7 @@ commands = black --check --diff . [testenv:mypy] -basepython = python3.10 +basepython = python3.11 deps = {[testenv]deps} mypy From 0c9e1c8deac18228394e6a486fd302354d14ea5c Mon Sep 17 00:00:00 2001 From: amedvede Date: Jun 01 2023 05:43:31 +0000 Subject: [PATCH 4/6] unretire plugin accept stg topic --- diff --git a/tests/plugins/test_pdc_unretire_packages.py b/tests/plugins/test_pdc_unretire_packages.py index c01f24c..cecb8bb 100644 --- a/tests/plugins/test_pdc_unretire_packages.py +++ b/tests/plugins/test_pdc_unretire_packages.py @@ -3,7 +3,7 @@ Unit tests for `toddler.plugins.pdc_unretire_packages` """ import datetime import logging -from unittest.mock import patch, Mock, call, MagicMock +from unittest.mock import call, MagicMock, Mock, patch import git import koji @@ -28,7 +28,14 @@ class TestAcceptTopic: """ assert toddler.accepts_topic("foo.bar") is False - @pytest.mark.parametrize("topic", ["io.pagure.prod.pagure.issue.new"]) + @pytest.mark.parametrize( + "topic", + [ + "io.pagure.*.pagure.issue.new", + "io.pagure.stg.pagure.issue.new", + "io.pagure.prod.pagure.issue.new", + ], + ) def test_accepts_topic_valid(self, topic, toddler): """ Assert that valid topics are accepted. @@ -269,8 +276,10 @@ class TestProcessTicket: }, "id": 55, } - message = "Remote repository doesn't exist or you do not have " \ - "the appropriate permissions to access it." + message = ( + "Remote repository doesn't exist or you do not have " + "the appropriate permissions to access it." + ) # self.toddler.git_repo.side_effect = git.GitCommandError with patch( "toddlers.plugins.pdc_unretire_packages.git.clone_repo", @@ -361,9 +370,7 @@ class TestProcessTicket: self.toddler.pagure_io.assign_maintainer_to_project.assert_called_with( namespace=namespace, repo=repo, maintainer_fas=issue["user"]["name"] ) - adjust_eol_pdc_mock.assert_called_with( - namespace, repo, tags_to_unblock - ) + adjust_eol_pdc_mock.assert_called_with(namespace, repo, tags_to_unblock) assert caplog.records[-1].message == "everything was alright!" @@ -420,18 +427,6 @@ class TestGetNamespaceAndRepo: """ self.toddler = pdc_unretire_packages.PDCUnretirePackages() - def test_get_namespace_and_repo_url_not_correct(self, caplog): - """ - Assert that list of elements is too short - """ - caplog.set_level(logging.INFO) - url = "https://src.fedoraproject.or" + "rpms/rubygem-logging" + ".git" - - assert self.toddler._get_namespace_and_repo(url) == (None, None) - assert caplog.records[-1].message == "Something wrong with url '{0}'".format( - url - ) - def test_get_namespace_and_repo_url_with_suffix(self): """ Assert that get namespace and repo works fine when url contain suffix. @@ -618,8 +613,10 @@ class TestIsPackageNotRetiredForReason: """ issue_id = 99 commit_msg = "legal issue" - close_issue_msg = f"Package was retire because of legal or license issue. " \ - f"Commit message '{commit_msg}'" + close_issue_msg = ( + f"Package was retire because of legal or license issue. " + f"Commit message '{commit_msg}'" + ) self.toddler.git_repo.get_last_commit_message.return_value = commit_msg @@ -661,14 +658,24 @@ class TestIsLastCommitDateCorrect: self.toddler.git_repo = Mock() self.toddler.pagure_io = Mock() + def test_is_last_commit_date_correct_date_date_is_none(self): + """ + Assert that method will return false if git won't be able to find a date. + """ + issue_id = 99 + self.toddler.git_repo.get_last_commit_date.return_value = None + assert self.toddler._is_last_commit_date_correct(issue_id) is False + def test_is_last_commit_date_correct_date_not_correct(self): """ Assert that last commit was made more than 8 weeks ago """ issue_id = 99 commit_date = 55 - close_issue_msg = "Package can not be unretired, because retire commit was made more " \ - "than 8 weeks ago." + close_issue_msg = ( + "Package can not be unretired, because retire commit was made more " + "than 8 weeks ago." + ) self.toddler.git_repo.get_last_commit_date.return_value = commit_date @@ -764,7 +771,9 @@ class TestVerifyBugzillaTicket: Assert that issue will be closed if fedora-review tag is missing on bugzilla. """ issue_id = 99 - issue_body = "Something https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2195577 ." + issue_body = ( + "Something https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2195577 ." + ) close_issue_msg = ( "Tag fedora-review is missing on bugzilla, get it and reopen ticket later." ) @@ -794,8 +803,10 @@ class TestVerifyBugzillaTicket: Assert that issue will be closed if fedora-review tag has wrong status. """ issue_id = 99 - issue_body = "Something https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2195577 " \ - "something" + issue_body = ( + "Something https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2195577 " + "something" + ) close_issue_msg = ( "Bug tag '{0}' don't have required status '{1}', " "but have '{2}' instead.\n" @@ -823,7 +834,9 @@ class TestVerifyBugzillaTicket: Assert that verify bugzilla process correctly. """ issue_id = 99 - issue_body = "Something https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2195577" + issue_body = ( + "Something https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2195577" + ) bug = Mock() bug.get_flags.return_value = [{"status": "+"}] @@ -1054,8 +1067,8 @@ class TestIsNeedToUnblockTagsOnKoji: ] assert ( - self.toddler._is_need_to_unblock_tags_on_koji(tags_to_unblock, repo) - is False + self.toddler._is_need_to_unblock_tags_on_koji(tags_to_unblock, repo) + is False ) @@ -1116,9 +1129,13 @@ class TestAdjustEolPdc: """ self.toddler = pdc_unretire_packages.PDCUnretirePackages() - @patch("toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages" - "._get_proper_eol_for_branches") - @patch("toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages._namespace_to_pdc") + @patch( + "toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages" + "._get_proper_eol_for_branches" + ) + @patch( + "toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages._namespace_to_pdc" + ) @patch("toddlers.utils.pdc.adjust_eol") def test_adjust_eol_pdc(self, adjust_eol_mock, namespace_to_pdc_mock, get_eol_mock): """ @@ -1147,12 +1164,17 @@ class TestAdjustEolPdc: eol=eol, ) - @patch("toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages" - "._get_proper_eol_for_branches") - @patch("toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages._namespace_to_pdc") + @patch( + "toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages" + "._get_proper_eol_for_branches" + ) + @patch( + "toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages._namespace_to_pdc" + ) @patch("toddlers.utils.pdc.adjust_eol") - def test_adjust_eol_pdc_few_branches(self, adjust_eol_mock, namespace_to_pdc_mock, - get_eol_mock): + def test_adjust_eol_pdc_few_branches( + self, adjust_eol_mock, namespace_to_pdc_mock, get_eol_mock + ): """ Assert that adjust_eol_pdc process correctly with few branches. """ @@ -1242,8 +1264,10 @@ class TestGetProperEolForBranches: """ self.toddler = pdc_unretire_packages.PDCUnretirePackages() - @patch("toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages." - "_get_last_fedora_version_branch_name") + @patch( + "toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages." + "_get_last_fedora_version_branch_name" + ) @patch("requests.get") def test_get_proper_eol_for_branches(self, get_request_mock, get_last_branch_mock): """ @@ -1259,19 +1283,38 @@ class TestGetProperEolForBranches: response.json.return_value = { "total": 2, "releases": [ - {"dist_tag": "f39", "state": "pending", "branch": "rawhide", "eol": None}, - {"dist_tag": "f37", "state": "pending", "branch": "rawhide", "eol": "2023-01-01"}, - {"dist_tag": "f38", "state": "pending", "branch": "rawhide", "eol": "2023-01-01"}, + { + "dist_tag": "f39", + "state": "pending", + "branch": "rawhide", + "eol": None, + }, + { + "dist_tag": "f37", + "state": "pending", + "branch": "rawhide", + "eol": "2023-01-01", + }, + { + "dist_tag": "f38", + "state": "pending", + "branch": "rawhide", + "eol": "2023-01-01", + }, ], } get_request_mock.return_value = response assert self.toddler._get_proper_eol_for_branches(tags_to_unblock) == result - @patch("toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages." - "_get_last_fedora_version_branch_name") + @patch( + "toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages." + "_get_last_fedora_version_branch_name" + ) @patch("requests.get") - def test_get_proper_eol_for_branches_few_branches(self, get_request_mock, get_last_branch_mock): + def test_get_proper_eol_for_branches_few_branches( + self, get_request_mock, get_last_branch_mock + ): """ Assert that get eol process correctly for few branches """ @@ -1287,9 +1330,24 @@ class TestGetProperEolForBranches: response.json.return_value = { "total": 2, "releases": [ - {"dist_tag": "f37", "state": "pending", "branch": "rawhide", "eol": "2023-01-01"}, - {"dist_tag": "f38", "state": "pending", "branch": "rawhide", "eol": "2023-01-02"}, - {"dist_tag": "f39", "state": "pending", "branch": "rawhide", "eol": None}, + { + "dist_tag": "f37", + "state": "pending", + "branch": "rawhide", + "eol": "2023-01-01", + }, + { + "dist_tag": "f38", + "state": "pending", + "branch": "rawhide", + "eol": "2023-01-02", + }, + { + "dist_tag": "f39", + "state": "pending", + "branch": "rawhide", + "eol": None, + }, ], } get_request_mock.return_value = response diff --git a/tests/utils/test_git.py b/tests/utils/test_git.py index 7ee6019..53145c0 100644 --- a/tests/utils/test_git.py +++ b/tests/utils/test_git.py @@ -115,7 +115,7 @@ class TestGitRepoGetLastCommitMessage: """ Assert that getting last commit message works correctly. """ - result = "this is commit message" + result = "this is commit message\n" commit_mock = Mock() commit_mock.message = "this is commit message\n" diff --git a/tests/utils/test_pagure.py b/tests/utils/test_pagure.py index ba85f5a..212a61e 100644 --- a/tests/utils/test_pagure.py +++ b/tests/utils/test_pagure.py @@ -34,7 +34,7 @@ class TestPagureSetPagure: Test initialization of pagure module without required config value. """ with pytest.raises( - ValueError, match=r"No pagure_url found in the configuration file" + ValueError, match=r"No pagure_url found in the configuration file" ): pagure.set_pagure({}) @@ -43,7 +43,7 @@ class TestPagureSetPagure: Test initialization of pagure module without required config value. """ with pytest.raises( - ValueError, match=r"No pagure_api_key found in the configuration file" + ValueError, match=r"No pagure_api_key found in the configuration file" ): config = {"pagure_url": "https://pagure.io"} pagure.set_pagure(config) @@ -201,7 +201,7 @@ class TestPagureCloseIssue: with pytest.raises(PagureError, match=expected_error): with patch( - "toddlers.utils.pagure.Pagure.add_comment_to_issue" + "toddlers.utils.pagure.Pagure.add_comment_to_issue" ) as comment_mock: comment_mock.return_value = True self.pagure.close_issue(issue_id, namespace, message, reason) @@ -1315,8 +1315,7 @@ class TestPagureIsProjectOrphaned: response_mock.ok = True keyword = "orphan" - data = {"epel_assignee": keyword, - "fedora_assignee": "some_fas"} + data = {"epel_assignee": keyword, "fedora_assignee": "some_fas"} response_mock.json.return_value = data @@ -1341,8 +1340,7 @@ class TestPagureIsProjectOrphaned: response_mock = Mock() response_mock.ok = True - data = {"epel_assignee": "some_fas", - "fedora_assignee": "some_fas"} + data = {"epel_assignee": "some_fas", "fedora_assignee": "some_fas"} response_mock.json.return_value = data @@ -1373,7 +1371,9 @@ class TestPagureIsProjectOrphaned: namespace = "namespace" repo = "repo" - expected_error = "Couldn't get project '{0}/{1}' maintainers.".format(namespace, repo) + expected_error = "Couldn't get project '{0}/{1}' maintainers.".format( + namespace, repo + ) with pytest.raises(PagureError, match=expected_error): self.pagure.is_project_orphaned(namespace, repo) @@ -1412,8 +1412,10 @@ class TestPagureAssignMaintainerToProject: namespace = "namespace" repo = "repo" maintainer_fas = "amedvede" - payload = {"EPEL Maintainer name": maintainer_fas, - "Maintainer name": maintainer_fas} + payload = { + "EPEL Maintainer name": maintainer_fas, + "Maintainer name": maintainer_fas, + } self.pagure.assign_maintainer_to_project(namespace, repo, maintainer_fas) @@ -1435,10 +1437,14 @@ class TestPagureAssignMaintainerToProject: namespace = "namespace" repo = "repo" maintainer_fas = "amedvede" - payload = {"EPEL Maintainer name": maintainer_fas, - "Maintainer name": maintainer_fas} + payload = { + "EPEL Maintainer name": maintainer_fas, + "Maintainer name": maintainer_fas, + } - expected_error = "Couldn't set new maintainer on project '{0}/{1}'".format(namespace, repo) + expected_error = "Couldn't set new maintainer on project '{0}/{1}'".format( + namespace, repo + ) with pytest.raises(PagureError, match=expected_error): self.pagure.assign_maintainer_to_project(namespace, repo, maintainer_fas) diff --git a/toddlers/plugins/pdc_unretire_packages.py b/toddlers/plugins/pdc_unretire_packages.py index a31e73c..e93b3bd 100644 --- a/toddlers/plugins/pdc_unretire_packages.py +++ b/toddlers/plugins/pdc_unretire_packages.py @@ -5,30 +5,29 @@ Authors: Anton Medvedev """ import argparse +import datetime import json import logging -import sys import re -import datetime +import sys import tempfile from typing import Optional -from git import GitCommandError - -import toml -import requests -import koji from fedora_messaging.api import Message +from git import GitCommandError +import koji from pagure_messages.issue_schema import IssueNewV1 +import requests +import toml from toddlers.base import ToddlerBase from toddlers.exceptions import ValidationError -from toddlers.utils import pagure, git, bugzilla_system, pdc +from toddlers.utils import bugzilla_system, git, pagure, pdc # Regex for bugzilla url BUGZILLA_URL_REGEX = r"https:\/\/bugzilla\.redhat\.com(.+)(id=\d{7})" # Regex for branches -BRANCHES_REGEX = r"f\d{2}|rawhide" +BRANCHES_REGEX = r"f\d{2}" # Where to look for pdc_unretire tickets PROJECT_NAMESPACE = "releng" # Keyword that will be searched for in the issue title @@ -60,10 +59,10 @@ class PDCUnretirePackages(ToddlerBase): name: str = "pdc_unretire_packages" - amqp_topics: dict = ["io.pagure.prod.pagure.issue.new"] + amqp_topics: list = ["io.pagure.*.pagure.issue.new"] # Dist-git base url - dist_git_base: str = "" + dist_git_base: Optional[str] = "" # TODO probably not working with mypy # PDC client object connected to pdc @@ -90,12 +89,16 @@ class PDCUnretirePackages(ToddlerBase): :returns: True if topic is accepted, False otherwise """ - return topic == "io.pagure.prod.pagure.issue.new" + if topic.startswith("io.pagure."): + if topic.endswith("pagure.issue.new"): + return True + + return False def process( - self, - config: dict, - message: Message, + self, + config: dict, + message: Message, ) -> None: """ Process a given message. @@ -173,7 +176,7 @@ class PDCUnretirePackages(ToddlerBase): # improve searching for package name end_of_keyword_in_title_index = ( - issue_title.lower().index(UNRETIRE_KEYWORD) + len(UNRETIRE_KEYWORD) + 1 + issue_title.lower().index(UNRETIRE_KEYWORD) + len(UNRETIRE_KEYWORD) + 1 ) inaccurate_package_name = issue_title[end_of_keyword_in_title_index:] @@ -221,8 +224,10 @@ class PDCUnretirePackages(ToddlerBase): try: self.git_repo = git.clone_repo(package_url, cloned_repo_name) except GitCommandError: - message = "Remote repository doesn't exist or you do not have the appropriate " \ - "permissions to access it." + message = ( + "Remote repository doesn't exist or you do not have the appropriate " + "permissions to access it." + ) _log.info(message) self.pagure_io.close_issue( issue_id, @@ -275,10 +280,10 @@ class PDCUnretirePackages(ToddlerBase): url = f"{self.dist_git_base}/{namespace}/{package_name}.git" if self._is_url_exist(url): return url - return + return None @staticmethod - def _get_namespace_and_repo(package_url: str) -> Optional[tuple]: + def _get_namespace_and_repo(package_url: str) -> tuple: """ Get namespace and package name from package url of exist package. @@ -295,16 +300,12 @@ class PDCUnretirePackages(ToddlerBase): list_of_elements = url_without_suffix.split("/") list_of_elements = [el for el in list_of_elements if el != ""] - if len(list_of_elements) == 4: - package_name = list_of_elements[-1] - namespace = list_of_elements[-2] - return namespace, package_name - else: - _log.info("Something wrong with url '{0}'".format(package_url)) - return None, None + package_name = list_of_elements[-1] + namespace = list_of_elements[-2] + return namespace, package_name def _verify_package_ready_for_unretirement( - self, issue_id: int, issue_body: str + self, issue_id: int, issue_body: str ) -> list: """ Verify that package is ready for unretirement. @@ -344,17 +345,19 @@ class PDCUnretirePackages(ToddlerBase): _log.info("Verifying that issue message doesn't contain forbidden keywords") if any( - re.search(forbidden_keyword, last_commit_message.lower()) - for forbidden_keyword in FORBIDDEN_KEYWORDS_FOR_COMMIT_MESSAGE + re.search(forbidden_keyword, str(last_commit_message).lower()) + for forbidden_keyword in FORBIDDEN_KEYWORDS_FOR_COMMIT_MESSAGE ): _log.info( "Commit message '{0}' contain forbidden keyword".format( - last_commit_message + str(last_commit_message) ) ) - message = "Package was retire because of legal or license issue. Commit message '{0}'" \ - "".format(last_commit_message) + message = ( + "Package was retire because of legal or license issue. Commit message '{0}'" + "".format(str(last_commit_message)) + ) if not DEVEL: self.pagure_io.close_issue( @@ -376,7 +379,13 @@ class PDCUnretirePackages(ToddlerBase): :returns: A bool value whether last commit was made more than 8 weeks ago """ last_commit_date = self.git_repo.get_last_commit_date() - last_commit_date_formatted = datetime.datetime.fromtimestamp(last_commit_date) + if last_commit_date is None: + return False + else: + last_commit_date_float = float(last_commit_date) + last_commit_date_formatted = datetime.datetime.fromtimestamp( + last_commit_date_float + ) current_time = datetime.datetime.now() time_diff = current_time - last_commit_date_formatted @@ -391,8 +400,10 @@ class PDCUnretirePackages(ToddlerBase): ) ) - message = "Package can not be unretired, because retire commit was made more than 8 " \ - "weeks ago." + message = ( + "Package can not be unretired, because retire commit was made more than 8 " + "weeks ago." + ) if not DEVEL: self.pagure_io.close_issue( @@ -455,6 +466,8 @@ class PDCUnretirePackages(ToddlerBase): _log.info("Getting bug with this id '{0}' data".format(bug_id)) bug = bugzilla_system.get_bug(bug_id) + if bug is None: + raise Exception("Bug was not found on bz.") except Exception as error: message = "Bugzilla can't get the bug by bug id, fix bugzilla url." if not DEVEL: @@ -517,20 +530,18 @@ class PDCUnretirePackages(ToddlerBase): :returns: A list with tags requester would like to unblock """ + last_branch_name = self._get_last_fedora_version_branch_name() tags = re.findall(BRANCHES_REGEX, issue_body) if len(tags) == 0: - tags = ["rawhide"] + tags = [last_branch_name] - last_branch_name = self._get_last_fedora_version_branch_name() + if last_branch_name not in tags: + tags.append(last_branch_name) - tags_to_unblock = list( - map(lambda el: el.replace("rawhide", last_branch_name), tags) - ) - - return tags_to_unblock + return tags @staticmethod - def _get_last_fedora_version_branch_name() -> str: + def _get_last_fedora_version_branch_name() -> Optional[str]: """ Get last fedora version branch name. @@ -552,11 +563,11 @@ class PDCUnretirePackages(ToddlerBase): if len(last_version) != 1: raise ValidationError("Not able to find a rawhide branch.") - last_version_name = last_version[0]["dist_tag"] + last_version_name = str(last_version[0]["dist_tag"]) return last_version_name def _is_need_to_unblock_tags_on_koji( - self, tags_to_unblock: list, repo: str + self, tags_to_unblock: list, repo: str ) -> bool: """ Check if at least any of the tags requested to be unblocked are really blocked. @@ -589,7 +600,7 @@ class PDCUnretirePackages(ToddlerBase): raise ValidationError("Package doesn't exist on koji.") def _unblock_tags_on_koji( - self, issue_id: int, tags_to_unblock: list, repo: str + self, issue_id: int, tags_to_unblock: list, repo: str ) -> None: """ Unblock certain tags for package. @@ -627,8 +638,12 @@ class PDCUnretirePackages(ToddlerBase): for key, value in branches_with_eol.items(): branch = key eol = value - pdc.adjust_eol(global_component=global_component, component_type=component_type, - branch=branch, eol=eol) + pdc.adjust_eol( + global_component=global_component, + component_type=component_type, + branch=branch, + eol=eol, + ) @staticmethod def _namespace_to_pdc(namespace): diff --git a/toddlers/utils/git.py b/toddlers/utils/git.py index c0750e8..391d6e1 100644 --- a/toddlers/utils/git.py +++ b/toddlers/utils/git.py @@ -3,7 +3,7 @@ Wrapper for git operations using GitPython library. Author: mkonecny@redhat.com """ -from typing import Optional +from typing import Optional, Union import git @@ -32,7 +32,7 @@ class GitRepo: repo (`git.Repo`): Inner representation of git repository """ - repo: Optional[git.Repo] = None + # repo: Optional[git.Repo] = None def __init__(self, repo: git.Repo) -> None: """ @@ -63,7 +63,7 @@ class GitRepo: return result - def get_last_commit_message(self) -> Optional[str]: + def get_last_commit_message(self) -> Union[str, bytes, None]: """ Returns the message of the last commit in master branch. @@ -75,7 +75,7 @@ class GitRepo: result = None if commit: - result = commit.message.split("\n")[0] + result = commit.message return result diff --git a/toddlers/utils/pagure.py b/toddlers/utils/pagure.py index 001ca5b..07abb88 100644 --- a/toddlers/utils/pagure.py +++ b/toddlers/utils/pagure.py @@ -139,7 +139,7 @@ class Pagure: ) def close_issue( - self, issue_id: int, namespace: str, message: str, reason: str = "Closed" + self, issue_id: int, namespace: str, message: str, reason: str = "Closed" ): """ Close the issue defined by the id with provided message and reason. @@ -308,14 +308,14 @@ class Pagure: ) def new_project( - self, - namespace: str, - repo: str, - description: str, - upstream_url: str, - default_branch: str, - initial_commit: bool = False, - alias: bool = False, + self, + namespace: str, + repo: str, + description: str, + upstream_url: str, + default_branch: str, + initial_commit: bool = False, + alias: bool = False, ) -> None: """ Create mew project in Pagure. @@ -433,12 +433,12 @@ class Pagure: ) def new_branch( - self, - namespace: str, - repo: str, - branch: str, - from_commit: str = "", - from_branch: str = "", + self, + namespace: str, + repo: str, + branch: str, + from_commit: str = "", + from_branch: str = "", ) -> None: """ Create a new branch in pagure repository. @@ -511,10 +511,10 @@ class Pagure: ) def set_monitoring_status( - self, - namespace: str, - repo: str, - monitoring_level: str, + self, + namespace: str, + repo: str, + monitoring_level: str, ) -> None: """ Set a monitoring status for pagure repository. This will work only on dist git. @@ -573,7 +573,7 @@ class Pagure: ) def change_project_main_admin( - self, namespace: str, repo: str, new_main_admin: str + self, namespace: str, repo: str, new_main_admin: str ) -> None: """ Change the main admin of a project in pagure. @@ -866,7 +866,9 @@ class Pagure: """ keyword = "orphan" - endpoint_url = "https://src.fedoraproject.org/_dg/bzoverrides/" + namespace + "/" + repo + endpoint_url = ( + "https://src.fedoraproject.org/_dg/bzoverrides/" + namespace + "/" + repo + ) headers = self.get_auth_header() log.debug("Getting project '{0}/{1}' maintainers.") @@ -874,7 +876,10 @@ class Pagure: if response.ok: result = response.json() - if result["epel_assignee"] == keyword or result["fedora_assignee"] == keyword: + if ( + result["epel_assignee"] == keyword + or result["fedora_assignee"] == keyword + ): return True else: return False @@ -883,9 +888,13 @@ class Pagure: "Error when retrieving project '{0}/{1}' maintainers. " "Got status_code '{2}'.".format(namespace, repo, response.status_code) ) - raise PagureError("Couldn't get project '{0}/{1}' maintainers.".format(namespace, repo)) + raise PagureError( + "Couldn't get project '{0}/{1}' maintainers.".format(namespace, repo) + ) - def assign_maintainer_to_project(self, namespace: str, repo: str, maintainer_fas: str) -> None: + def assign_maintainer_to_project( + self, namespace: str, repo: str, maintainer_fas: str + ) -> None: """ Assign maintainer to project. Working only with dist-git @@ -898,10 +907,14 @@ class Pagure: Raises: `toddlers.utils.exceptions.PagureError``: When setting project maintainer fails. """ - endpoint_url = "https://src.fedoraproject.org/_dg/bzoverrides/" + namespace + "/" + repo + endpoint_url = ( + "https://src.fedoraproject.org/_dg/bzoverrides/" + namespace + "/" + repo + ) headers = self.get_auth_header() - payload = {"EPEL Maintainer name": maintainer_fas, - "Maintainer name": maintainer_fas} + payload = { + "EPEL Maintainer name": maintainer_fas, + "Maintainer name": maintainer_fas, + } log.debug( "Setting new maintainer to '{0}' for project '{1}/{2}'".format( @@ -915,10 +928,10 @@ class Pagure: if response.status_code != 200: log.error( "Error when setting new maintainer on project '{0}/{1}'. Got status_code '{2}'." - "".format( - namespace, repo, response.status_code - ) + "".format(namespace, repo, response.status_code) ) raise PagureError( - "Couldn't set new maintainer on project '{0}/{1}'".format(namespace, repo) + "Couldn't set new maintainer on project '{0}/{1}'".format( + namespace, repo + ) ) diff --git a/toddlers/utils/pdc.py b/toddlers/utils/pdc.py index 955dafb..4a4b79d 100644 --- a/toddlers/utils/pdc.py +++ b/toddlers/utils/pdc.py @@ -90,7 +90,7 @@ def get_sla_for_branch( def new_sla_to_branch( - sla_name: str, eol: str, global_component: str, branch: str, branch_type: str + sla_name: str, eol: str, global_component: str, branch: str, branch_type: str ) -> None: """ Create a new SLA to branch mapping in PDC. Does nothing if SLA already exists. @@ -268,10 +268,7 @@ def adjust_eol(global_component, component_type, branch, eol): pdc = get_pdc() - payload = { - "eol": eol, - "branch_active": True - } + payload = {"eol": eol, "branch_active": True} for branch_sla in existing_branch_slas: pdc["component-branch-slas"][branch_sla["id"]]._(payload) diff --git a/toddlers/utils/requests.py b/toddlers/utils/requests.py index 521722a..bf68f7a 100644 --- a/toddlers/utils/requests.py +++ b/toddlers/utils/requests.py @@ -5,7 +5,7 @@ from typing import Any, Optional, Tuple, Union import requests from urllib3.util import retry -from urllib3.util.timeout import Timeout as timeout# type: ignore +from urllib3.util.timeout import Timeout as timeout # type: ignore # mypy couldn't find the typing information for default @@ -84,8 +84,9 @@ def make_session( BACKOFF_MAX = retry_conf.pop("BACKOFF_MAX") + retry.Retry.DEFAULT_BACKOFF_MAX = BACKOFF_MAX retry_conf_obj = retry.Retry(**retry_conf) - retry_conf_obj.BACKOFF_MAX = BACKOFF_MAX + # retry_conf_obj.BACKOFF_MAX = BACKOFF_MAX http_adapter = TimeoutHTTPAdapter(timeout=timeout, max_retries=retry_conf_obj) # type: ignore From 7668bb4019d178ed35220f8ffe42216b162ab34a Mon Sep 17 00:00:00 2001 From: Mattia Verga Date: Jun 01 2023 06:46:43 +0000 Subject: [PATCH 5/6] Change BZ ticket status on repository creation Signed-off-by: Mattia Verga --- diff --git a/tests/utils/test_bugzilla_system.py b/tests/utils/test_bugzilla_system.py index ced86eb..2eedd7e 100644 --- a/tests/utils/test_bugzilla_system.py +++ b/tests/utils/test_bugzilla_system.py @@ -1103,6 +1103,70 @@ class TestCommentOnBug: mock_bz_call.assert_called_with(bug.addcomment, args=(comment,)) +class TestChangeBugStatus: + """Test class for `toddlers.utils.bugzilla_system.change_bug_status` function.""" + + @patch("toddlers.utils.bugzilla_system.get_bug") + def test_change_ticket_status(self, mock_get_bug): + """Assert that change is called without issue.""" + bug_id = "100" + status = "POST" + comment = "comment" + + bug = Mock() + + mock_get_bug.return_value = bug + + toddlers.utils.bugzilla_system.change_bug_status(bug_id, status, comment) + + mock_get_bug.assert_called_with(bug_id) + bug.setstatus.assert_called_with(status, comment) + + @patch("toddlers.utils.bugzilla_system.get_bug") + @patch("toddlers.utils.bugzilla_system.execute_bugzilla_call") + def test_comment_on_bug_xmlrpc_fault(self, mock_bz_call, mock_get_bug): + """Assert that XMLRPC fault is handled.""" + bug_id = "100" + status = "POST" + comment = "comment" + + mock_bz_call.side_effect = xmlrpc.client.Fault(50, "Fault") + + bug = Mock() + mock_get_bug.return_value = bug + + with pytest.raises(xmlrpc.client.Fault) as exc: + toddlers.utils.bugzilla_system.change_bug_status(bug_id, status, comment) + + assert exc.value.args == (comment, 50, "Fault") + + mock_get_bug.assert_called_with(bug_id) + mock_bz_call.assert_called_with(bug.setstatus, args=(status, comment,)) + + @patch("toddlers.utils.bugzilla_system.get_bug") + @patch("toddlers.utils.bugzilla_system.execute_bugzilla_call") + def test_comment_on_bug_protocol_error(self, mock_bz_call, mock_get_bug): + """Assert that protocol error is handled.""" + bug_id = "100" + status = "POST" + comment = "comment" + + mock_bz_call.side_effect = xmlrpc.client.ProtocolError( + "Error", 10, "Error message", {} + ) + + bug = Mock() + mock_get_bug.return_value = bug + + with pytest.raises(xmlrpc.client.ProtocolError) as exc: + toddlers.utils.bugzilla_system.change_bug_status(bug_id, status, comment) + + assert exc.value.args == ("ProtocolError", 10, "Error message") + + mock_get_bug.assert_called_with(bug_id) + mock_bz_call.assert_called_with(bug.setstatus, args=(status, comment,)) + + class TestExecuteBugzillaCall: """Test class for `toddlers.utils.bugzilla_system.execute_bugzilla_call` function.""" diff --git a/toddlers/plugins/scm_request_processor.py b/toddlers/plugins/scm_request_processor.py index 660b8aa..1339fe1 100644 --- a/toddlers/plugins/scm_request_processor.py +++ b/toddlers/plugins/scm_request_processor.py @@ -843,7 +843,7 @@ class SCMRequestProcessor(ToddlerBase): reason="Processed", ) if bug_id: - bugzilla_system.comment_on_bug(bug_id, new_branch_comment) + bugzilla_system.change_bug_status(bug_id, 'POST', new_branch_comment) def validate_review_bug( self, diff --git a/toddlers/utils/bugzilla_system.py b/toddlers/utils/bugzilla_system.py index 5c6bee6..79343b9 100644 --- a/toddlers/utils/bugzilla_system.py +++ b/toddlers/utils/bugzilla_system.py @@ -619,6 +619,40 @@ def comment_on_bug(bug_id: str, comment: str) -> None: _log.error("Bug '{}' not found!".format(bug_id)) +def change_bug_status(bug_id: str, status: str, + comment: Optional[str] = None) -> None: + """ + Change status to bug on Bugzilla. + + Commonly-used values are ASSIGNED, MODIFIED, POST and NEEDINFO. + + Params: + bug_id: Identifier of the bug + status: Status to set on bug. + comment: Comment to post + + Raises: + `xmlrpc.client.Fault`: When the operation fails. + `xmlrpc.client.ProtocolError`: When communication with bugzilla fails. + """ + _log.info("Setting status `%s` to `%s`", status, bug_id) + + bug = get_bug(bug_id) + + if bug: + try: + execute_bugzilla_call(bug.setstatus, args=(status, comment,)) + except xmlrpc.client.Fault as e: + # Output something useful in args + e.args = (comment, e.faultCode, e.faultString) + raise + except xmlrpc.client.ProtocolError as e: + e.args = ("ProtocolError", e.errcode, e.errmsg) + raise + else: + _log.error("Bug '{}' not found!".format(bug_id)) + + R = TypeVar("R") From c9ee172d9899db5d90c26a7e2b0f6e4b12ecc93a Mon Sep 17 00:00:00 2001 From: Mattia Verga Date: Jun 01 2023 08:04:51 +0000 Subject: [PATCH 6/6] Update scm_request_processor tests Signed-off-by: Mattia Verga --- diff --git a/tests/plugins/test_scm_request_processor.py b/tests/plugins/test_scm_request_processor.py index 11214b7..d01e9cf 100644 --- a/tests/plugins/test_scm_request_processor.py +++ b/tests/plugins/test_scm_request_processor.py @@ -2038,7 +2038,7 @@ class TestCreateNewBranch: reason="Processed", ) - mock_bz.comment_on_bug.assert_called_with(bug_id, message) + mock_bz.change_bug_status.assert_called_with(bug_id, "POST", message) class TestValidateReviewBug: diff --git a/tests/utils/test_bugzilla_system.py b/tests/utils/test_bugzilla_system.py index 2eedd7e..a3874f2 100644 --- a/tests/utils/test_bugzilla_system.py +++ b/tests/utils/test_bugzilla_system.py @@ -1164,7 +1164,13 @@ class TestChangeBugStatus: assert exc.value.args == ("ProtocolError", 10, "Error message") mock_get_bug.assert_called_with(bug_id) - mock_bz_call.assert_called_with(bug.setstatus, args=(status, comment,)) + mock_bz_call.assert_called_with( + bug.setstatus, + args=( + status, + comment, + ) + ) class TestExecuteBugzillaCall: diff --git a/toddlers/plugins/scm_request_processor.py b/toddlers/plugins/scm_request_processor.py index 1339fe1..eb471e4 100644 --- a/toddlers/plugins/scm_request_processor.py +++ b/toddlers/plugins/scm_request_processor.py @@ -843,7 +843,7 @@ class SCMRequestProcessor(ToddlerBase): reason="Processed", ) if bug_id: - bugzilla_system.change_bug_status(bug_id, 'POST', new_branch_comment) + bugzilla_system.change_bug_status(bug_id, "POST", new_branch_comment) def validate_review_bug( self,