From bd9578a6b904319b8562febe601e6e2436e2c920 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Kone=C4=8Dn=C3=BD?= Date: Fri, 26 Aug 2022 13:59:39 +0200 Subject: [PATCH] Remove PDC check from scm_request_processor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PDC check is blocking further processing of the ticket. For example if PDC entries are created, but there is error during dist-git calls, it will currently just fails and the rest of the process needs to be done manually. Without the PDC check the processing will continue and the PDC will not create duplicate entries. Signed-off-by: Michal Konečný --- tests/plugins/test_scm_request_processor.py | 144 +------------------- toddlers/plugins/scm_request_processor.py | 42 +----- 2 files changed, 3 insertions(+), 183 deletions(-) diff --git a/tests/plugins/test_scm_request_processor.py b/tests/plugins/test_scm_request_processor.py index 1ec93de..3de1aed 100644 --- a/tests/plugins/test_scm_request_processor.py +++ b/tests/plugins/test_scm_request_processor.py @@ -1127,55 +1127,6 @@ class TestCreateNewRepo: reason="Invalid", ) - @patch("toddlers.utils.pdc.get_branch") - @patch( - "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug" - ) - def test_create_new_repo_branch_exist( - self, mock_validate_review_bug, mock_pdc_get_branch - ): - """ - Assert that ticket will be closed when branch already exist in PDC. - """ - issue = {"id": 100, "user": {"name": "zlopez"}} - - repo = "repo" - branch = "rawhide" - namespace = "rpms" - bug_id = "123" - action = "new_repo" - sls = {"rawhide": "2050-06-01"} - monitor = "monitor" - exception = False - json = { - "repo": repo, - "branch": branch, - "namespace": namespace, - "bug_id": bug_id, - "action": action, - "sls": sls, - "monitor": monitor, - "exception": exception, - } - self.toddler.dist_git.get_project.return_value = None - mock_pdc_get_branch.return_value = branch - - self.toddler.create_new_repo(issue, json) - - mock_validate_review_bug.assert_called_with( - bug_id, repo, branch, namespace=namespace, pagure_user="zlopez" - ) - - self.toddler.dist_git.get_project.assert_called_with(namespace, repo) - mock_pdc_get_branch.assert_called_with(repo, branch, namespace.rstrip("s")) - - self.toddler.pagure_io.close_issue.assert_called_with( - 100, - namespace=scm_request_processor.PROJECT_NAMESPACE, - message="The PDC branch already exists", - reason="Invalid", - ) - @pytest.mark.parametrize( "namespace, branch", [ @@ -1223,7 +1174,6 @@ class TestCreateNewRepo: "users": {"admin": [user], "commit": [], "ticket": []} } self.toddler.validation_comment = "valid" - mock_pdc.get_branch.return_value = None # Method to test self.toddler.create_new_repo(issue, json) @@ -1240,7 +1190,6 @@ class TestCreateNewRepo: namespace, repo, "zlopez" ) - mock_pdc.get_branch.assert_called_with(repo, branch, namespace.rstrip("s")) mock_pdc.new_global_component.assert_called_with( repo, "{0}/{1}/{2}".format(dist_git_url, namespace, repo) ) @@ -1260,8 +1209,7 @@ class TestCreateNewRepo: reason="Processed", ) - @patch("toddlers.plugins.scm_request_processor.pdc") - def test_create_new_repo_tests_namespace(self, mock_pdc): + def test_create_new_repo_tests_namespace(self): """ Assert that ticket will be processed when everything is in order and namespace is set to tests. @@ -1301,7 +1249,6 @@ class TestCreateNewRepo: "users": {"admin": [user], "commit": [], "ticket": []} } self.toddler.validation_comment = "valid" - mock_pdc.get_branch.return_value = None self.toddler.create_new_repo(issue, json) @@ -1321,8 +1268,6 @@ class TestCreateNewRepo: namespace, repo, "zlopez" ) - mock_pdc.get_branch.assert_called_with(repo, branch, namespace.rstrip("s")) - message = "The Pagure repository was created at {0}/{1}/{2}".format( dist_git_url, namespace, repo ) @@ -1371,7 +1316,6 @@ class TestCreateNewRepo: dist_git_url = "https://src.fp.o" self.toddler.dist_git.get_project.return_value = None self.toddler.dist_git._pagure_url = dist_git_url - mock_pdc.get_branch.return_value = None self.toddler.create_new_repo(issue, json) @@ -1391,12 +1335,6 @@ class TestCreateNewRepo: namespace, repo, "zlopez" ) - mock_pdc.get_branch.assert_has_calls( - [ - call(repo, "rawhide", namespace.rstrip("s")), - call(repo, branch, namespace.rstrip("s")), - ] - ) mock_pdc.new_global_component.assert_called_with( repo, "{0}/{1}/{2}".format(dist_git_url, namespace, repo) ) @@ -1462,7 +1400,6 @@ class TestCreateNewRepo: dist_git_url = "https://src.fp.o" self.toddler.dist_git.get_project.return_value = None self.toddler.dist_git._pagure_url = dist_git_url - mock_pdc.get_branch.return_value = None self.toddler.create_new_repo(issue, json) @@ -1482,7 +1419,6 @@ class TestCreateNewRepo: namespace, repo, "zlopez" ) - mock_pdc.get_branch.assert_called_with(repo, branch, namespace.rstrip("s")) mock_pdc.new_global_component.assert_called_with( repo, "{0}/{1}/{2}".format(dist_git_url, namespace, repo) ) @@ -1611,59 +1547,8 @@ class TestCreateNewBranch: reason="Invalid", ) - @patch("toddlers.plugins.scm_request_processor.pdc") - def test_create_new_branch_branch_exists(self, mock_pdc): - """ - Assert that ticket will be closed if branch already exists in PDC. - """ - issue = {"id": 100, "user": {"name": "zlopez"}} - - repo = "repo" - branch = "rawhide" - namespace = "rpms" - action = "new_branch" - sls = {"rawhide": "2050-06-01"} - json = { - "repo": repo, - "branch": branch, - "namespace": namespace, - "action": action, - "sls": sls, - } - - self.toddler.create_new_branch(issue, json) - - # Asserts - self.toddler.dist_git.get_project_contributors.assert_called_with( - namespace, repo - ) - - mock_pdc.get_branch.assert_called_with( - repo, branch, namespace.strip().rstrip("s") - ) - - message = ( - "The branch in PDC already exists, you can now create it yourself as follows:\n" - "Check in the project's settings if you have activated the git hook preventing " - "new git branches from being created and if you did, de-activate it.\n" - "Then simply run in cloned repository:\n" - "``git checkout -b && git push -u origin ``.\n" - "```` is the name of the branch you requested. \n" - "You only need to do this once and you can then use fedpkg as you normally do." - ) - - self.toddler.pagure_io.close_issue.assert_called_with( - 100, - namespace=scm_request_processor.PROJECT_NAMESPACE, - message=message, - reason="Invalid", - ) - @patch("toddlers.plugins.scm_request_processor.fedora_account") - @patch("toddlers.plugins.scm_request_processor.pdc") - def test_create_new_branch_requester_is_not_maintainer( - self, mock_pdc, mock_fedora_account - ): + def test_create_new_branch_requester_is_not_maintainer(self, mock_fedora_account): """ Assert that ticket will be closed if requester is not a maintainer of package. """ @@ -1685,7 +1570,6 @@ class TestCreateNewBranch: "users": {"admin": [], "commit": [], "collaborators": []}, "groups": {"admin": ["group"], "commit": [], "collaborators": []}, } - mock_pdc.get_branch.return_value = None mock_fedora_account.user_member_of.return_value = False self.toddler.create_new_branch(issue, json) @@ -1695,10 +1579,6 @@ class TestCreateNewBranch: namespace, repo ) - mock_pdc.get_branch.assert_called_with( - repo, branch, namespace.strip().rstrip("s") - ) - mock_fedora_account.user_member_of.assert_called_with( mock_fedora_account.get_user_by_username(), "group" ) @@ -1741,7 +1621,6 @@ class TestCreateNewBranch: } self.toddler.dist_git.get_default_branch.return_value = None self.toddler.dist_git._pagure_url = "https://fp.o" - mock_pdc.get_branch.return_value = None mock_fedora_account.user_member_of.return_value = True mock_dir = MagicMock() @@ -1756,10 +1635,6 @@ class TestCreateNewBranch: namespace, repo ) - mock_pdc.get_branch.assert_called_with( - repo, branch, namespace.strip().rstrip("s") - ) - mock_fedora_account.user_member_of.assert_called_with( mock_fedora_account.get_user_by_username(), "group" ) @@ -1824,7 +1699,6 @@ class TestCreateNewBranch: } self.toddler.dist_git.get_default_branch.return_value = default_branch self.toddler.dist_git._pagure_url = "https://fp.o" - mock_pdc.get_branch.return_value = None mock_fedora_account.user_member_of.return_value = True mock_dir = MagicMock() @@ -1843,10 +1717,6 @@ class TestCreateNewBranch: namespace, repo ) - mock_pdc.get_branch.assert_called_with( - repo, branch, namespace.strip().rstrip("s") - ) - mock_fedora_account.user_member_of.assert_called_with( mock_fedora_account.get_user_by_username(), "group" ) @@ -1916,7 +1786,6 @@ class TestCreateNewBranch: "groups": {"admin": ["group"], "commit": [], "collaborators": []}, } self.toddler.dist_git._pagure_url = "https://fp.o" - mock_pdc.get_branch.return_value = None mock_fedora_account.user_member_of.return_value = True self.toddler.create_new_branch(issue, json) @@ -1926,10 +1795,6 @@ class TestCreateNewBranch: namespace, repo ) - mock_pdc.get_branch.assert_called_with( - repo, branch, namespace.strip().rstrip("s") - ) - mock_fedora_account.user_member_of.assert_called_with( mock_fedora_account.get_user_by_username(), "group" ) @@ -1991,7 +1856,6 @@ class TestCreateNewBranch: "groups": {"admin": ["group"], "commit": [], "collaborators": []}, } self.toddler.dist_git._pagure_url = "https://fp.o" - mock_pdc.get_branch.return_value = None mock_fedora_account.user_member_of.return_value = True self.toddler.create_new_branch(issue, json) @@ -2001,10 +1865,6 @@ class TestCreateNewBranch: namespace, repo ) - mock_pdc.get_branch.assert_called_with( - repo, branch, namespace.strip().rstrip("s") - ) - mock_fedora_account.user_member_of.assert_called_with( mock_fedora_account.get_user_by_username(), "group" ) diff --git a/toddlers/plugins/scm_request_processor.py b/toddlers/plugins/scm_request_processor.py index c8c2bf1..546d6c7 100644 --- a/toddlers/plugins/scm_request_processor.py +++ b/toddlers/plugins/scm_request_processor.py @@ -578,26 +578,6 @@ class SCMRequestProcessor(ToddlerBase): reason="Invalid", ) return - existing_branch = None - if branch_name != default_branch: - _log.info( - "- Checking if {0} default branch already exists in PDC.".format( - default_branch - ) - ) - existing_branch = pdc.get_branch(repo, default_branch, component_type) - - _log.info("- Checking if {0} already exists in PDC.".format(branch_name)) - branch = pdc.get_branch(repo, branch_name, component_type) - - if existing_branch or branch: - self.pagure_io.close_issue( - issue["id"], - namespace=PROJECT_NAMESPACE, - message="The PDC branch already exists", - reason="Invalid", - ) - return _log.info("Ticket passed all validations. Creating repository.") # Create the PDC SLA entry @@ -727,26 +707,6 @@ class SCMRequestProcessor(ToddlerBase): # Pagure uses plural names for namespaces, but PDC does not use the # plural version for branch types branch_type = namespace.strip().rstrip("s") - _log.info("- Checking if {0} already exists in PDC.".format(branch_name)) - pdc_branch = pdc.get_branch(repo, branch_name, branch_type) - if pdc_branch: - ticket_text = ( - "The branch in PDC already exists, you can now create it yourself as follows:\n" - "Check in the project's settings if you have activated the git hook preventing " - "new git branches from being created and if you did, de-activate it.\n" - "Then simply run in cloned repository:\n" - "``git checkout -b && git push -u origin ``.\n" - "```` is the name of the branch you requested. \n" - "You only need to do this once and you can then use fedpkg as you normally do." - ) - self.pagure_io.close_issue( - issue["id"], - namespace=PROJECT_NAMESPACE, - message=ticket_text, - reason="Invalid", - ) - return - issue_owner = issue["user"]["name"] # Check if the branch requestor is one of the maintainers or part of the groups @@ -797,7 +757,7 @@ class SCMRequestProcessor(ToddlerBase): ) return - _log.info("Ticket passed all validations. Creating repository.") + _log.info("Ticket passed all validations. Creating branch in repository.") # Create the PDC entry dist_git_url = "{0}/{1}/{2}".format( self.dist_git._pagure_url.rstrip("/"), namespace, repo