#156 [WIP] pdc_unretire_packages plugin
Closed 2 years ago by amedvede. Opened 2 years ago by amedvede.
fedora-infra/ amedvede/toddlers feature_plugin  into  main

The added file is too large to be shown here, see it at: tests/plugins/test_pdc_unretire_packages.py
@@ -0,0 +1,751 @@ 

+ """

+ When in issue person ask to untretire a package, this script will make it automatically.

I would just use something like: "This is a script to automate unretirement of package automatically, when ticket is created."

+ 

+ Authors:    Anton Medvedev <amedvede@redhat.com>

+ 

+ """

+ import argparse

+ import json

+ import logging

+ import sys

+ import shutil

+ import re

+ import datetime

+ import toml

+ import requests

+ import koji

+ import git as git_python

+ 

+ from fedora_messaging.api import Message

+ from pagure_messages.issue_schema import IssueNewV1

+ 

+ from toddlers.base import ToddlerBase

+ from toddlers.exceptions import ValidationError

+ from toddlers.utils import pagure, git, bugzilla_system, 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"

+ # Where to look for pdc_unretire tickets

+ PROJECT_NAMESPACE = "releng"

+ # Keyword that will be searched for in the issue title

+ UNRETIRE_KEYWORD = "unretire"

+ # Forbidden keywords for commit message

+ FORBIDDEN_KEYWORDS_FOR_COMMIT_MESSAGE = ["legal", "license"]

+ # Time difference limit not getting Bugzilla url

+ TIME_DIFFERENCE_LIMIT = 56  # 8 weeks in days

+ # Package retirement process url

+ PACKAGE_RETIREMENT_PROCESS_URL = "https://docs.fedoraproject.org/en-US/package-maintainers" \

+                                  "/Package_Retirement_Process/#claiming"

+ # Fedora review bugzilla tag

+ FEDORA_REVIEW_TAG_NAME = "fedora-review"

+ # Koji hub url

+ KOJIHUB_URL = "https://koji.fedoraproject.org/kojihub"

+ # Set certain parts not ot run if it's not in production

+ IS_IN_PRODUCTION = True

Usually this is solved by opposite way. You will have DEBUG or DEVEL global variable.

+ 

+ _log = logging.getLogger(__name__)

+ 

+ 

+ class PDCUnretiredPackages(ToddlerBase):

+     """

+     Listen for new tickets in https://pagure.io/releng/issues

+     and process then, either by unretiring a package or rejecting the ticket

+     """

+ 

+     name: str = "pdc_unretire_packages"

+ 

+     amqp_topics: dict = [

+         "io.pagure.prod.pagure.issue.new"

+     ]

+ 

+     # PDC client object connected to pdc

+     pdc_client: pdc.PDCClient

+ 

+     # Pagure object connected to pagure.io

+     pagure_io: pagure.Pagure

+ 

+     # Pagure object conected to dist-git

+     dist_git: pagure.Pagure

+ 

+     # Git repo object

+     git_repo: git.GitRepo

+ 

+     # Koji session object

+     koji_session: koji.ClientSession

+ 

+     def accepts_topic(self, topic: str) -> bool:

+         """

+         Returns a boolean whether this toddler is interested in messages

+         from this specific topic.

+ 

+         :arg topic: Topic to check

+ 

+         :returns: True if topic is accepted, False otherwise

+         """

+         return topic == "io.pagure.prod.pagure.issue.new"

+ 

+     def process(

+             self,

+             config: dict,

+             message: Message,

+     ) -> None:

+         """

+         Process a given message.

+ 

+         :arg config: Toddlers configuration

+         :arg message: Message to process

+         """

+         _log.debug(

+             "Processing message:\n{0}".format(json.dumps(message.body, indent=2))

+         )

+         project_name = message.body["project"]["fullname"]

+ 

+         if project_name != PROJECT_NAMESPACE:

+             _log.info(

+                 "The message doesn't belong to project {0}. Skipping message.".format(

+                     PROJECT_NAMESPACE

+                 )

+             )

+             return

+ 

+         issue = message.body["issue"]

+ 

+         if IS_IN_PRODUCTION:

+             if issue["status"] != "Open":

+                 _log.info(

+                     "The issue {0} is not open. Skipping message.".format(issue["id"])

+                 )

+                 return

+ 

+         issue_title = issue["title"]

+ 

+         if UNRETIRE_KEYWORD not in issue_title.lower():

+             _log.info("The issue doesn't contain keyword '{0}' in the title '{1}'".format(

+                 UNRETIRE_KEYWORD,

+                 issue_title

+             )

+             )

+             return

+ 

+         _log.info("Setting up connection to PDC")

+         self.pdc_client = pdc.pdc_client_for_config(config)

+ 

+         _log.info("Setting up connection to Pagure")

+         self.pagure_io = pagure.set_pagure(config)

+ 

+         _log.info("Setting up connection to Bugzilla")

+         bugzilla_system.set_bz(config)

+ 

+         _log.info("Setting up session with Koji")

+         self.koji_session = koji.ClientSession(KOJIHUB_URL)

+ 

+         self.process_ticket(issue)

+ 

+     def process_ticket(self, issue: dict) -> None:

+         """

+         Process a single ticket

+ 

+         :arg issue: A dictionary containing the issue

+         """

+         _log.info("Handling pagure releng ticket '{0}'".format(issue["full_url"]))

+ 

+         issue_title = issue["title"]

+         issue_body = issue["content"].strip("`").strip("\n")

+         issue_url = issue["full_url"]

+         issue_opener = issue["user"]["name"]

+ 

+         # improve searching for package name

+         end_of_keyword_in_title_index = issue_title.lower().index(UNRETIRE_KEYWORD) + len(UNRETIRE_KEYWORD) + 1

+ 

+         inaccurate_package_name = issue_title[end_of_keyword_in_title_index:]

+         list_of_words_in_package_name = [word.strip() for word in inaccurate_package_name.split(" ")]

+ 

+         if len(list_of_words_in_package_name) > 1:

+             _log.info(

+                 "Package name '{0}' has two or more words".format(inaccurate_package_name)

+             )

+             return

+ 

+         inaccurate_package_name = list_of_words_in_package_name[0]

+         cloned_repo_name = "{0}_repo".format(inaccurate_package_name.replace("/", "_"))

+         package_url = self._get_package_url(inaccurate_package_name)

+ 

+         if package_url == "not_found":

+             _log.info(

+                 "Package with this name '{0}' wasn't found on dist-git".format(inaccurate_package_name)

+             )

+             return

+ 

+         namespace, package_name = self._get_package_name_and_namespace_from_package_url(package_url)

+ 

+         _log.info("Cloning repo into dir with name '{0}'".format(cloned_repo_name))

+         try:

+             self.git_repo = git.clone_repo(package_url, cloned_repo_name)

You have import git as git_python, so this should be git_python.clone_repo.

I see you also importing git from toddlers.utils. Maybe it would be better to improve the git module instead of importing git_python directly.

+         except git_python.GitCommandError:

+             _log.info(

+                 "Can't find git repo with url '{0}'".format(package_url)

+             )

+             self._clean_cloned_repo(cloned_repo_name)

+             return

+ 

+         tags_to_unblock = self._verify_package_ready_for_unretirement(issue=issue)

+ 

+         self._revert_the_retirement_commit(issue_url=issue_url)

+ 

+         self._unblock_the_package_on_koji(tags_to_unblock, package_name)

+ 

+         self._verify_the_package_is_not_orphaned(namespace, package_name, requester_fas=issue_opener)

+ 

+         self._update_pdc()

+ 

+         self._clean_cloned_repo(cloned_repo_name)

+ 

+         _log.info("everything was alright!")

+         return

+ 

+     def _get_package_url(self, package_name: str) -> str:

+         """

+         Return a package url.

+ 

+         :arg package_name: A package name with or without namespace

+ 

+         :returns: Package dist-git url or 'not_found' if url doesn't exist.

+         """

+         base = "https://src.fedoraproject.org/"

+         package_namespaces = ("rpms/", "modules/", "container/")

+ 

+         if package_name.startswith(package_namespaces):

+             url = f"{base}/{package_name}.git"

+             if self._is_url_exist(url):

+                 return url

+         else:

+             for namespace in package_namespaces:

+                 url = f"{base}/{namespace}/{package_name}.git"

+                 if self._is_url_exist(url):

+                     return url

+         return "not_found"

It's better to use None or false, you can then compare it just like if not package_url:

+ 

+     @staticmethod

+     def _get_package_name_and_namespace_from_package_url(package_url: str) -> tuple:

+         """

+         Get namespace and package name from package url of exist package.

+ 

+         :arg package_url: A package url

+ 

+         :returns: A tuple with package namespace and package name

+         """

+         url_without_suffix = package_url.removesuffix(".git")

+ 

+         list_of_elements = url_without_suffix.split('/')

+         list_of_elements = [el for el in list_of_elements if el != '']

+ 

+         package_name = list_of_elements[-1]

+         namespace = list_of_elements[-2]

+         return namespace, package_name

+ 

+     def _verify_package_ready_for_unretirement(self, issue: dict) -> list:

+         """

+         Verify that package is ready for unretirement.

+ 

+         :arg issue: A dict with issue data.

+ 

+         :returns: A list with tags requester ask to unblock.

+         """

+         _log.info("Verifying that package is ready for unretirement")

+ 

+         issue_id = issue["id"]

+         issue_body = issue["content"].strip("`").strip("\n")

This is already done in process_ticket, no need for doing it twice.

+ 

+         if not self._verify_package_was_not_retire_for_reason():

+             raise ValidationError(

How do you handle these errors? In current solution it will just return the message to queue and process it again.

+                 "Package was retired for a reason: legal of license issue."

+             )

+ 

+         if not self._verify_that_last_commit_was_made_less_than_8_weeks_ago():

+             raise ValidationError(

+                 "Last commit was made more than 8 weeks ago."

+             )

+ 

+         if not self._ensure_that_bugzilla_was_filled_to_review_the_package_for_unretirement(issue_id, issue_body):

No need for these long names of methods, just verify_bugzilla_ticket and good comment in the method should be enough.

+             raise ValidationError(

+                 "Package on bugzilla was not verified to be unretired."

+             )

+ 

+         tags_to_unblock = self._get_tags_to_unblock(issue_body)

+         return tags_to_unblock

+ 

+     def _verify_package_was_not_retire_for_reason(self) -> bool:

Typo in the name of the method retire -> retired. And it's good habit to name methods that return bool is_something. In this case it would be _is_package_retired_for_reason.

+         """

+         Verify that commit message does not contain forbidden keywords

+ 

+         :returns: Bool value whether package have legal issues

+         """

+         last_commit_message = self._get_last_commit_message()

+ 

+         _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):

+             _log.info(

+                 "Commit message '{0}' contain forbidden keyword".format(last_commit_message)

+             )

+             return False

+         else:

+             return True

+ 

+     def _get_last_commit_message(self) -> str:

This method should be in utils/git.py.

+         """

+         Get last commit message.

+ 

+         :returns: A string with last commit message.

+         """

+         last_commit = self.git_repo.repo.commit()

+         last_commit_message = last_commit.message.split("\n")[0]

+         return last_commit_message

+ 

+     def _verify_that_last_commit_was_made_less_than_8_weeks_ago(self) -> bool:

+         """

+         Verify that last commit was made less than 8 weeks ago.

+ 

+         :returns: A bool value whether last commit was made more than 8 weeks ago

+         """

+         last_commit_date = self._get_last_commit_date()

+ 

+         time_difference_between_commit_date_and_current_date_in_days = \

+             self._get_time_difference_between_last_commit_date_and_current_date(last_commit_date)

+ 

+         if time_difference_between_commit_date_and_current_date_in_days <= TIME_DIFFERENCE_LIMIT:

+             return True

+         else:

+             _log.info(

+                 "Last commit date is more then 8 weeks (56 days) it's actually {}".format(

+                     time_difference_between_commit_date_and_current_date_in_days

+                 )

+             )

+             return False

+ 

+     def _get_last_commit_date(self) -> int:

Should be in utils/git.py to be used by other modules.

+         """

+         Get last commit date.

+ 

+         :returns: A int which can be converted to datetime object

+         """

+         last_commit = self.git_repo.repo.commit()

+         last_commit_date = last_commit.committed_date

+         return last_commit_date

+ 

+     @staticmethod

+     def _get_time_difference_between_last_commit_date_and_current_date(last_commit_date: int) -> int:

This could be in _verify_that_last_commit_was_made_less_than_8_weeks_ago, no need to create a method for something that it just used once in one specific method.

+         """

+         Get time difference in days between last commit date and current date.

+ 

+         :arg last_commit_date: A last commit date in seconds

+ 

+         :returns: Time difference in days

+         """

+         last_commit_date_formatted = datetime.datetime.fromtimestamp(last_commit_date)

+         current_time = datetime.datetime.now()

+         time_difference = current_time - last_commit_date_formatted

+         time_difference_in_days = time_difference.days

+         return time_difference_in_days

+ 

+     def _ensure_that_bugzilla_was_filled_to_review_the_package_for_unretirement(

+             self, issue_id: int, issue_body: str) -> bool:

+         """

+         Check whether package exist on bugzilla and whether it has a tags

+ 

+         :arg issue_id: Int value of issue id

+         :arg issue_body: Str with body of issue

+ 

+         :returns: A bool value whether check succeed

+         """

+         _log.info("Getting bugzilla url from issue body")

+ 

+         bugzilla_url = self._get_bugzilla_url(issue_body)

+ 

+         if bugzilla_url == "not_found":

+             _log.info(

+                 "Issue body doesn't contain a bugzilla url"

+             )

+ 

+             comment = "The package is retired for more than 8 Weeks as per {0}\n" \

+                       "We require a re-review to unretire this package.\n\n" \

+                       "Once you have BZ with fedora_review+ please reopen this ticket." \

+                       "".format(PACKAGE_RETIREMENT_PROCESS_URL)

+ 

+             if IS_IN_PRODUCTION:

+                 self.pagure_io.add_comment_to_issue(

+                     issue_id=issue_id,

+                     namespace=PROJECT_NAMESPACE,

+                     comment=comment,

+                 )

+ 

+             _log.info(

+                 "Closing issue, because bugzilla url wasn't found"

+             )

+ 

+             message = "Bugzilla url is missing, please add it and reopen ticket"

+ 

+             if IS_IN_PRODUCTION:

+                 self.pagure_io.close_issue(

+                     issue_id=issue_id,

+                     namespace=PROJECT_NAMESPACE,

+                     message=message,

+                     reason="Get back later"

+                 )

+             return False

+ 

+         bug_id = bugzilla_url.split("id=")[1]

+         try:

+             _log.info(

+                 "Getting bug with this id '{0}' data".format(bug_id)

+             )

+ 

+             bug = bugzilla_system.get_bug(bug_id)

+         except Exception as error:

+             raise ValidationError(

+                 "The Bugzilla bug could not be verified. The following "

+                 "error was encountered: {0}".format(str(error))

+             )

+ 

+         try:

+             _log.info("Getting {0} tag from bug".format(FEDORA_REVIEW_TAG_NAME))

+ 

+             fedora_review_tag = bug.get_flags(FEDORA_REVIEW_TAG_NAME)

+             print(fedora_review_tag)

+             fedora_review_tag_status = fedora_review_tag[0]["status"]

+ 

+             if fedora_review_tag_status != "+":

+                 message = "Bug tag '{0}' don't have required status '{1}', " \

+                           "but have '{2}' instead.\n" \

+                           "Closing ticket".format(FEDORA_REVIEW_TAG_NAME, "+", fedora_review_tag_status)

+                 _log.info(message)

+ 

+                 if IS_IN_PRODUCTION:

+                     self.pagure_io.close_issue(

+                         issue_id=issue_id,

+                         namespace=PROJECT_NAMESPACE,

+                         message=message,

+                         reason="Closed"

+                     )

+                 return False

+ 

+         except TypeError:

+             raise ValidationError(

+                 "`{0}` tag is missing.".format(FEDORA_REVIEW_TAG_NAME)

+             )

+ 

+         return True

+ 

+     @staticmethod

+     def _get_bugzilla_url(issue_body: str) -> str:

No need for separate method just to get bugzilla url.

+         """

+         Get bugzilla url from issue body.

+ 

+         :arg issue_body: A text of issue

+ 

+         :returns: String with bugzilla url or 'not_found' if url didn't mention in text

+         """

+         match = re.search(BUGZILLA_URL_REGEX, issue_body)

+         if match:

+             bugzilla_url = match.group()

+             return bugzilla_url

+         return "not_found"

+ 

+     def _get_tags_to_unblock(self, issue_body: str) -> list:

+         """

+         Get tags requester would like to unblock.

+ 

+         :arg issue_body: A str value with text of issue body

+ 

+         :returns: A list with tags requester would like to unblock

+         """

+         tags = self._get_tags(issue_body)

+ 

+         last_branch_name = self._get_last_fedora_version_branch_name()

+ 

+         tags_for_koji_unblock = list(map(lambda el: el.replace("rawhide", last_branch_name), tags))

+ 

+         return tags_for_koji_unblock

+ 

+     @staticmethod

+     def _get_tags(issue_body: str) -> list:

+         """

+         Get branches that need to be unretire from issue body.

+ 

+         :arg issue_body: A text of issue

+ 

+         :returns: List with branches or [] if branches didn't mention in text

+         """

+         branches = re.findall(BRANCHES_REGEX, issue_body)

+         if len(branches) != 0:

+             return branches

+         return ["rawhide"]

+ 

+     @staticmethod

+     def _get_last_fedora_version_branch_name() -> str:

Wouldn't this be better to just read from configuration, instead of calling bodhi url each time a message is processed?

+         """

+         Get last fedora version branch name.

+ 

+         :returns: String with name

+         """

+         bodhi_url = "https://bodhi.fedoraproject.org/releases/"

+         response = requests.get(url=bodhi_url)

+         count_of_version_on_bodhi = response.json()["total"]

+ 

+         params = {"rows_per_page": count_of_version_on_bodhi}

+         response = requests.get(url=bodhi_url, params=params)

+ 

+         versions = response.json()["releases"]

+         last_version = [version for version in versions

+                         if version["state"] == "pending" and version["branch"] == "rawhide"]

+         if len(last_version) != 1:

+             raise ValidationError("Not able to find a rawhide branch.")

+ 

+         last_version = last_version[0]

+         last_version_name = last_version["dist_tag"]

+         return last_version_name

+ 

+     def _revert_the_retirement_commit(self, issue_url: str) -> None:

I would move this to utils/git.py as revert_last_commit method.

+         """

+         Revert retire commit.

+ 

+         :arg issue_url: A url of ticket

+         """

+         _log.info("Reverting retire commit")

+         message = "Unretirement request: {0}".format(issue_url)

+ 

+         commit_before_retire = self.git_repo.repo.commit("HEAD~1")

+         commit_hash = commit_before_retire.hexsha

+ 

+         self.git_repo.repo.git.execute(["git", "checkout", f"{commit_hash}", "."])

+         self.git_repo.repo.index.commit(message=message)

+ 

+         if IS_IN_PRODUCTION:

+             origin = self.git_repo.repo.remote("origin")

+             origin.push()

+ 

+     def _unblock_the_package_on_koji(self, tags_to_unblock: list, package_name: str) -> None:

+         """

+         Unblocking tags of package on koji.

+ 

+         :arg tags_to_unblock: A list of tags to unblock

+         :arg package_name: A str of package name

+         """

+         if not self._verify_that_branches_for_unblock_is_blocked_on_koji(

+                 tags_to_unblock, package_name):

+             raise ValidationError(

+                 "Tags that are required to be unblock are not blocked."

+             )

+ 

+         self._unblock_certain_tags_for_package_on_koji(tags_to_unblock, package_name)

+ 

+     def _verify_that_branches_for_unblock_is_blocked_on_koji(

+             self, tags_to_unblock: list, package_name: str) -> bool:

+         """

+         Verify branches, that need be unblocked, are blocked

+ 

+         :arg tags_to_unblock: List of branch names

+         :arg package_name: Name of package

+ 

+         :returns: Bool value if it's verified

+         """

+         _log.info("Verifying that tags are blocked on koji.")

+         try:

+             package_tags = self.koji_session.listTags(package=package_name)

+             if not package_tags:

+                 _log.info("Package '{0}' doesn't have a tags".format(package_name))

+                 return False

+             tags_that_suppose_to_be_blocked = [tag for tag in package_tags if tag["name"] in tags_to_unblock]

+             return all([tag["blocked"] for tag in tags_that_suppose_to_be_blocked])

+         except koji.GenericError:

+             _log.info("Package '{0}' doesn't exist".format(package_name))

+             return False

+ 

+     def _unblock_certain_tags_for_package_on_koji(self, tags_to_unblock: list, package_name: str) -> None:

+         """

+         Unblock certain tags for package.

+ 

+         :arg tags_to_unblock: List of tags to be unblocked

+         :arg package_name: Package name without namespace

+         """

+         _log.info("Unblocking tags in koji.")

+         if IS_IN_PRODUCTION:

+             try:

+                 for tag in tags_to_unblock:

+                     self.koji_session.packageListUnblock(taginfo=tag, pkginfo=package_name)

+             except koji.GenericError:

+                 _log.info("Not able to unblock a tag of package.")

+ 

+     def _verify_the_package_is_not_orphaned(self, namespace: str, package_name: str, requester_fas: str) -> None:

This could be probably moved to utils/pagure.py.

+         """

+         Verify that package is not orphaned.

+ 

+         :arg namespace: Namespace of package.

+         :arg package_name: Name of package.

+         :arg requester_fas: FAS of issue opener.

+         """

+         _log.info("Verifying that package is not orphaned.")

+         keyword = "orphan"

+         endpoint_url = "https://src.fedoraproject.org/_dg/bzoverrides/" + namespace + "/" + package_name

+         response = requests.get(endpoint_url)

+         response_data = response.json()

+         if response_data["epel_assignee"] == keyword or response_data["fedora_assignee"] == keyword:

+             _log.info("Need to assign ticket opener.")

+             self._assign_requestor_to_package(namespace, package_name, requester_fas)

+         else:

+             _log.info("No need to assign ticket opener.")

+ 

+     @staticmethod

+     def _assign_requestor_to_package(namespace: str, package_name: str, requester_fas: str) -> None:

This could be probably moved to utils/pagure.py.

+         """

+         Assign requester to package.

+ 

+         :arg namespace: Namespace of package.

+         :arg package_name: Name of package.

+         :arg requester_fas: Fas of issues opener.

+         """

+         _log.info("Assigning requester to package.")

+ 

+         # Here should be API key from config.

+         # headers = {"Authorization": "token {0}".format(api_key)}

+         endpoint_url = "https://src.fedoraproject.org/_dg/bzoverrides/" + namespace + "/" + package_name

+         data = {"EPEL Maintainer name": requester_fas,

+                 "Maintainer name": requester_fas}

+ 

+         # response = requests.post(url=endpoint_url, headers=headers, json=data)

+         response = requests.post(url=endpoint_url, json=data)

+ 

+         if response.status_code == 200:

+             _log.info("Requestor was assign with {0} status code.".format(response.status_code))

+         else:

+             raise ValidationError("Requester was not assign. Status code is {0}.".format(response.status_code))

+ 

+     def _update_pdc(self):

+         # TODO get PDC token and write the func

+         pass

+ 

+     @staticmethod

+     def _clean_cloned_repo(cloned_repo_name: str) -> bool:

You should do all the work using the TemporaryDir and just clone the repo there, this way you don't need to clean the repo manually.

+         """

+         Clean cloned repo dir.

+ 

+         :arg cloned_repo_name: A name of cloned dir

+ 

+         :returns: Bool value if successfully cleaned

+ 

+         :raises: 'OSError' When not able to clean dir

+         """

+         _log.info("Cleaning cloned repo dir with name '{0}'".format(cloned_repo_name))

+         try:

+             shutil.rmtree(cloned_repo_name)

+             return True

+         except OSError:

+             raise OSError(

+                 "Program not able to remove dir '{0}'".format(cloned_repo_name)

+             )

+ 

+     @staticmethod

+     def _is_url_exist(url: str) -> bool:

+         """

+         Check whether url exist.

+ 

+         :arg url: Url that might exist

+ 

+         :returns: True if url exist, otherwise False

+         """

+         response = requests.get(url=url)

+         return response.status_code == 200

+ 

+ 

+ def _get_arguments(args):

+     """Load and parse the CLI arguments.

+ 

+     :arg args: Script arguments

+ 

+     :returns: Parsed arguments

+     """

+     parser = argparse.ArgumentParser(

+         description="Processor for PDC unretire packages, handling tickets from '{}'".format(

+             PROJECT_NAMESPACE

+         )

+     )

+ 

+     parser.add_argument(

+         "ticket",

+         type=int,

+         help="Number of ticket to process",

+     )

+ 

+     parser.add_argument(

+         "--config",

+         help="Configuration file",

+     )

+ 

+     parser.add_argument(

+         "--debug",

+         action="store_const",

+         dest="log_level",

+         const=logging.DEBUG,

+         default=logging.INFO,

+         help="Enable debugging output",

+     )

+     return parser.parse_args(args)

+ 

+ 

+ def _setup_logging(log_level: int) -> None:

+     """

+     Setup the logging level.

+ 

+     :arg log_level: Log level to set

+     """

+     handlers = []

+ 

+     _log.setLevel(log_level)

+     # We want all messages logged at level INFO or lower to be printed to stdout

+     info_handler = logging.StreamHandler(stream=sys.stdout)

+     handlers.append(info_handler)

+ 

+     if log_level == logging.INFO:

+         # In normal operation, don't decorate messages

+         for handler in handlers:

+             handler.setFormatter(logging.Formatter("%(message)s"))

+ 

+     logging.basicConfig(level=log_level, handlers=handlers)

+ 

+ 

+ def main(args):

+     """Main function"""

+     args = _get_arguments(args)

+     _setup_logging(log_level=args.log_level)

+     _log.info("hello i'm starting work")

+ 

+     config = toml.load(args.config)

+     parsed_config = config.get("consumer_config", {}).get("default", {})

+     parsed_config.update(

+         config.get("consumer_config", {}).get("pdc_unretire_packages", {})

+     )

+ 

+     ticket = args.ticket

+ 

+     pagure_io = pagure.set_pagure(parsed_config)

+     issue = pagure_io.get_issue(ticket, PROJECT_NAMESPACE)

+ 

+     # Convert issue to message

+     message = IssueNewV1()

+     message.body["issue"] = issue

+     message.body["project"] = {"fullname": PROJECT_NAMESPACE}

+     _log.debug("Message prepared: {}".format(message.body))

+ 

+     PDCUnretiredPackages().process(

+         config=parsed_config,

+         message=message,

+     )

+ 

+ 

+ if __name__ == "__main__":

+     try:

+         main(sys.argv[1:])

+     except KeyboardInterrupt:

+         pass

Add new pdc_unretire_packages plugin.

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/84a3097392bb4726ad9e03a0976803f5

I would just use something like: "This is a script to automate unretirement of package automatically, when ticket is created."

You have import git as git_python, so this should be git_python.clone_repo.

I see you also importing git from toddlers.utils. Maybe it would be better to improve the git module instead of importing git_python directly.

It's better to use None or false, you can then compare it just like if not package_url:

This is already done in process_ticket, no need for doing it twice.

No need for these long names of methods, just verify_bugzilla_ticket and good comment in the method should be enough.

Typo in the name of the method retire -> retired. And it's good habit to name methods that return bool is_something. In this case it would be _is_package_retired_for_reason.

How do you handle these errors? In current solution it will just return the message to queue and process it again.

This method should be in utils/git.py.

Should be in utils/git.py to be used by other modules.

This could be in _verify_that_last_commit_was_made_less_than_8_weeks_ago, no need to create a method for something that it just used once in one specific method.

No need for separate method just to get bugzilla url.

Usually this is solved by opposite way. You will have DEBUG or DEVEL global variable.

Wouldn't this be better to just read from configuration, instead of calling bodhi url each time a message is processed?

I would move this to utils/git.py as revert_last_commit method.

This could be probably moved to utils/pagure.py.

This could be probably moved to utils/pagure.py.

You should do all the work using the TemporaryDir and just clone the repo there, this way you don't need to clean the repo manually.

I added a few comments to this PR and also there are failing tests.

Pull-Request has been closed by amedvede

2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/040c434a335749d1bb4f009e640cdcc9