From f74362a73caf2727699a8cbe8295765eca118c97 Mon Sep 17 00:00:00 2001 From: Laura Barcziova Date: Mon, 24 Jun 2024 14:52:16 +0200 Subject: [PATCH] Utilise LocalProjectBuilder for LP initalisation With this change, for copr builds also try to not clone the repo. Related to #2411 --- .../worker/helpers/build/koji_build.py | 1 + packit_service/worker/helpers/job_helper.py | 30 +++++++++++++++---- packit_service/worker/mixin.py | 25 ++++++++++++---- tests/integration/test_dg_commit.py | 23 +++++++------- tests/integration/test_issue_comment.py | 3 +- tests/integration/test_new_hotness_update.py | 3 +- tests/integration/test_pr_comment.py | 6 ++-- tests/integration/test_release_event.py | 20 +++++++++---- tests/unit/test_build_helper.py | 16 +++++----- tests/unit/test_steve.py | 4 +-- 10 files changed, 86 insertions(+), 45 deletions(-) diff --git a/packit_service/worker/helpers/build/koji_build.py b/packit_service/worker/helpers/build/koji_build.py index 7b2d443fe..c0c124647 100644 --- a/packit_service/worker/helpers/build/koji_build.py +++ b/packit_service/worker/helpers/build/koji_build.py @@ -36,6 +36,7 @@ class KojiBuildJobHelper(BaseBuildJobHelper): job_type_test = None status_name_build: str = "koji-build" status_name_test: str = None + require_git_repo_in_local_project: bool = True def __init__( self, diff --git a/packit_service/worker/helpers/job_helper.py b/packit_service/worker/helpers/job_helper.py index da7b1dd05..0d66ce8e4 100644 --- a/packit_service/worker/helpers/job_helper.py +++ b/packit_service/worker/helpers/job_helper.py @@ -12,7 +12,12 @@ from packit.api import PackitAPI from packit.config import JobConfig from packit.config.package_config import PackageConfig -from packit.local_project import LocalProject +from packit.local_project import ( + LocalProject, + LocalProjectBuilder, + CALCULATE, + NOT_TO_CALCULATE, +) from packit.utils.repo import RepositoryCache from packit_service.config import Deployment, ServiceConfig from packit_service.models import ( @@ -28,6 +33,8 @@ class BaseJobHelper: + require_git_repo_in_local_project: bool = False + def __init__( self, service_config: ServiceConfig, @@ -78,11 +85,7 @@ def msg_retrigger(self) -> str: @property def local_project(self) -> LocalProject: if self._local_project is None: - self._local_project = LocalProject( - git_project=self.project, - working_dir=self.service_config.command_handler_work_dir, - ref=self.metadata.git_ref, - pr_id=self.metadata.pr_id, + builder = LocalProjectBuilder( cache=( RepositoryCache( cache_path=self.service_config.repository_cache, @@ -91,7 +94,22 @@ def local_project(self) -> LocalProject: if self.service_config.repository_cache else None ), + ) + self._local_project = builder.build( + git_project=self.project, + working_dir=self.service_config.command_handler_work_dir, + ref=self.metadata.git_ref, + pr_id=self.metadata.pr_id, merge_pr=self.package_config.merge_pr_in_ci, + git_url=CALCULATE, + repo_name=CALCULATE, + full_name=CALCULATE, + namespace=CALCULATE, + git_repo=( + CALCULATE + if self.require_git_repo_in_local_project + else NOT_TO_CALCULATE + ), ) return self._local_project diff --git a/packit_service/worker/mixin.py b/packit_service/worker/mixin.py index eed7e9718..9425f28d8 100644 --- a/packit_service/worker/mixin.py +++ b/packit_service/worker/mixin.py @@ -13,7 +13,7 @@ from ogr.abstract import Issue from packit.api import PackitAPI -from packit.local_project import LocalProject +from packit.local_project import LocalProject, LocalProjectBuilder, CALCULATE from packit.utils.repo import RepositoryCache from ogr.abstract import GitProject, PullRequest @@ -214,10 +214,9 @@ class LocalProjectMixin(Config): @property def local_project(self) -> LocalProject: + if not self._local_project: - kwargs = dict( - working_dir=Path(self.service_config.command_handler_work_dir) - / SANDCASTLE_LOCAL_PROJECT_DIR, + builder = LocalProjectBuilder( cache=( RepositoryCache( cache_path=self.service_config.repository_cache, @@ -225,13 +224,27 @@ def local_project(self) -> LocalProject: ) if self.service_config.repository_cache else None - ), + ) ) + working_dir = Path( + Path(self.service_config.command_handler_work_dir) + / SANDCASTLE_LOCAL_PROJECT_DIR + ) + kwargs = dict( + repo_name=CALCULATE, + full_name=CALCULATE, + namespace=CALCULATE, + working_dir=working_dir, + git_repo=CALCULATE, + ) + if self.project: kwargs["git_project"] = self.project else: kwargs["git_url"] = self.project_url - self._local_project = LocalProject(**kwargs) + + self._local_project = builder.build(**kwargs) + return self._local_project diff --git a/tests/integration/test_dg_commit.py b/tests/integration/test_dg_commit.py index 988345ea2..4c55a284f 100644 --- a/tests/integration/test_dg_commit.py +++ b/tests/integration/test_dg_commit.py @@ -6,9 +6,10 @@ import pytest from celery.canvas import group from flexmock import flexmock + +from ogr.abstract import PRStatus from ogr.services.github import GithubProject from ogr.services.pagure import PagureProject - from packit.api import PackitAPI from packit.config import ( CommonPackageConfig, @@ -19,7 +20,7 @@ PackageConfig, ) from packit.exceptions import PackitException -from packit.local_project import LocalProject +from packit.local_project import LocalProjectBuilder from packit.utils.koji_helper import KojiHelper from packit.utils.repo import RepositoryCache from packit_service.config import PackageConfigGetter, ProjectToSync, ServiceConfig @@ -43,8 +44,6 @@ run_downstream_koji_build, run_sync_from_downstream_handler, ) - -from ogr.abstract import PRStatus from tests.spellbook import DATA_DIR, first_dict_value, get_parameters_from_results @@ -108,7 +107,7 @@ def test_sync_from_downstream(): ) ) - flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: None) flexmock(RepositoryCache).should_call("__init__").once() flexmock(group).should_receive("apply_async").once() flexmock(Pushgateway).should_receive("push").times(2).and_return() @@ -281,7 +280,7 @@ def set_koji_name(name): {"name": koji_target} ) - flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: None) flexmock(group).should_receive("apply_async").once() flexmock(Pushgateway).should_receive("push").times(2).and_return() flexmock(PackitAPI).should_receive("build").with_args( @@ -369,7 +368,7 @@ def test_downstream_koji_build_failure_no_issue(): ) flexmock(Pushgateway).should_receive("push").times(1).and_return() - flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: None) flexmock(group).should_receive("apply_async").once() flexmock(PackitAPI).should_receive("build").with_args( dist_git_branch="main", @@ -459,7 +458,7 @@ def test_downstream_koji_build_failure_issue_created(): flexmock(grouped_targets=[koji_build]) ) flexmock(Pushgateway).should_receive("push").times(2).and_return() - flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: None) flexmock(group).should_receive("apply_async").once() flexmock(PackitAPI).should_receive("build").with_args( dist_git_branch="main", @@ -557,7 +556,7 @@ def test_downstream_koji_build_failure_issue_comment(): ) flexmock(Pushgateway).should_receive("push").times(2).and_return() - flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: None) flexmock(group).should_receive("apply_async").once() flexmock(PackitAPI).should_receive("build").with_args( dist_git_branch="main", @@ -631,7 +630,7 @@ def test_downstream_koji_build_no_config(): ) flexmock(Pushgateway).should_receive("push").times(1).and_return() - flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: None) flexmock(group).should_receive("apply_async").times(0) processing_results = SteveJobs().process_message(distgit_commit_event()) @@ -735,7 +734,7 @@ def test_downstream_koji_build_where_multiple_branches_defined(jobs_config): flexmock(grouped_targets=[koji_build]) ) - flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: None) flexmock(group).should_receive("apply_async").once() flexmock(Pushgateway).should_receive("push").times(2).and_return() flexmock(PackitAPI).should_receive("build").with_args( @@ -838,7 +837,7 @@ def test_do_not_run_downstream_koji_build_for_a_different_branch(jobs_config): ) flexmock(Pushgateway).should_receive("push").times(1).and_return() - flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: None) flexmock(PackitAPI).should_receive("build").times(0) processing_results = SteveJobs().process_message(distgit_commit_event()) diff --git a/tests/integration/test_issue_comment.py b/tests/integration/test_issue_comment.py index 28c79e011..ead5ea945 100644 --- a/tests/integration/test_issue_comment.py +++ b/tests/integration/test_issue_comment.py @@ -19,7 +19,7 @@ from packit.config import JobConfigTriggerType, PackageConfig from packit.distgit import DistGit from packit.exceptions import PackitException -from packit.local_project import LocalProject +from packit.local_project import LocalProject, LocalProjectBuilder from packit.utils.koji_helper import KojiHelper from packit_service.config import ServiceConfig, PackageConfigGetter from packit_service.constants import COMMENT_REACTION, TASK_ACCEPTED @@ -135,6 +135,7 @@ def mock_comment(request): ) flexmock(project_class).should_receive("get_releases").and_return([gr]) flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: flexmock()) lp = flexmock(git_project=flexmock(default_branch="main")) lp.working_dir = "" flexmock(DistGit).should_receive("local_project").and_return(lp) diff --git a/tests/integration/test_new_hotness_update.py b/tests/integration/test_new_hotness_update.py index 4f7348492..515e91a23 100644 --- a/tests/integration/test_new_hotness_update.py +++ b/tests/integration/test_new_hotness_update.py @@ -14,7 +14,7 @@ from packit.config import JobConfigTriggerType from packit.config.aliases import get_branches from packit.distgit import DistGit -from packit.local_project import LocalProject +from packit.local_project import LocalProject, LocalProjectBuilder from packit_service.config import ServiceConfig from packit_service.models import ( ProjectEventModelType, @@ -126,6 +126,7 @@ def test_new_hotness_update(new_hotness_update, sync_release_model): default_branch="main", ) lp = flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: lp) lp.working_dir = "" lp.git_project = project flexmock(DistGit).should_receive("local_project").and_return(lp) diff --git a/tests/integration/test_pr_comment.py b/tests/integration/test_pr_comment.py index 22ecbeafd..057558a62 100644 --- a/tests/integration/test_pr_comment.py +++ b/tests/integration/test_pr_comment.py @@ -23,7 +23,7 @@ from packit.copr_helper import CoprHelper from packit.distgit import DistGit from packit.exceptions import PackitConfigException -from packit.local_project import LocalProject +from packit.local_project import LocalProject, LocalProjectBuilder from packit.upstream import Upstream from packit.utils.koji_helper import KojiHelper from packit_service.config import ServiceConfig @@ -191,6 +191,7 @@ def mock_pr_comment_functionality(request): project_url="https://github.com/packit-service/hello-world", ).and_return(db_project_object) flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: flexmock()) flexmock(Allowlist, check_and_report=True) @@ -2426,7 +2427,7 @@ def test_koji_build_retrigger_via_dist_git_pr_comment(pagure_pr_comment_added): ) flexmock(DownstreamKojiBuildHandler).should_receive("pre_check").and_return(True) - + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: flexmock()) flexmock(LocalProject, refresh_the_arguments=lambda: None) flexmock(celery_group).should_receive("apply_async").once() flexmock(PackitAPI).should_receive("build").with_args( @@ -2636,6 +2637,7 @@ def test_pull_from_upstream_retrigger_via_dist_git_pr_comment(pagure_pr_comment_ default_branch="main", ) lp = flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: lp) lp.working_dir = "" lp.git_project = project flexmock(DistGit).should_receive("local_project").and_return(lp) diff --git a/tests/integration/test_release_event.py b/tests/integration/test_release_event.py index 6765fe3fa..f20ad1538 100644 --- a/tests/integration/test_release_event.py +++ b/tests/integration/test_release_event.py @@ -14,7 +14,7 @@ from packit.config.aliases import get_branches from packit.distgit import DistGit from packit.exceptions import PackitDownloadFailedException -from packit.local_project import LocalProject +from packit.local_project import LocalProject, LocalProjectBuilder from packit.pkgtool import PkgTool from packit_service import sentry_integration from packit_service.config import ServiceConfig @@ -169,7 +169,8 @@ def test_dist_git_push_release_handle(github_release_webhook, propose_downstream is_private=lambda: False, default_branch="main", ) - lp = flexmock(LocalProject, refresh_the_arguments=lambda: None) + lp = flexmock() + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: lp) lp.working_dir = "" lp.git_project = project flexmock(DistGit).should_receive("local_project").and_return(lp) @@ -295,6 +296,7 @@ def test_dist_git_push_release_handle_multiple_branches( is_private=lambda: False, default_branch="main", ) + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: flexmock()) flexmock(LocalProject, refresh_the_arguments=lambda: None) flexmock(LocalProject).should_receive("git_repo").and_return( flexmock( @@ -437,6 +439,8 @@ def test_dist_git_push_release_handle_one_failed( .mock() ) project.should_receive("get_issue_list").and_return([]) + lp = flexmock() + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: lp) flexmock(LocalProject, refresh_the_arguments=lambda: None) flexmock(LocalProject).should_receive("git_repo").and_return( flexmock( @@ -616,7 +620,8 @@ def test_dist_git_push_release_handle_all_failed( .mock() ) project.should_receive("get_issue_list").and_return([]) - lp = flexmock(LocalProject, refresh_the_arguments=lambda: None) + lp = flexmock() + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: lp) lp.git_project = project lp.git_url = "https://src.fedoraproject.org/rpms/hello-world.git" lp.working_dir = "" @@ -722,7 +727,8 @@ def test_retry_propose_downstream_task( default_branch="main", ) - lp = flexmock(LocalProject, refresh_the_arguments=lambda: None) + lp = flexmock() + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: lp) lp.git_project = project lp.working_dir = "" flexmock(DistGit).should_receive("local_project").and_return(lp) @@ -844,7 +850,8 @@ def test_dont_retry_propose_downstream_task( ) project.should_receive("get_issue_list").and_return([]).once() - lp = flexmock(LocalProject, refresh_the_arguments=lambda: None) + lp = flexmock() + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: lp) lp.git_project = project lp.git_url = "https://src.fedoraproject.org/rpms/hello-world.git" lp.working_dir = "" @@ -972,7 +979,8 @@ def test_dist_git_push_release_failed_issue_creation_disabled( .never() .mock() ) - lp = flexmock(LocalProject, refresh_the_arguments=lambda: None) + lp = flexmock() + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: lp) lp.git_project = project lp.git_url = "https://src.fedoraproject.org/rpms/hello-world.git" lp.working_dir = "" diff --git a/tests/unit/test_build_helper.py b/tests/unit/test_build_helper.py index 20fdb782c..caffe49d1 100644 --- a/tests/unit/test_build_helper.py +++ b/tests/unit/test_build_helper.py @@ -1,15 +1,9 @@ # Copyright Contributors to the Packit project. # SPDX-License-Identifier: MIT -from pathlib import Path import pytest from flexmock import flexmock -from packit.config.notifications import ( - NotificationsConfig, - FailureCommentNotificationsConfig, -) -from packit.copr_helper import CoprHelper from packit.config import ( CommonPackageConfig, JobConfig, @@ -18,6 +12,11 @@ PackageConfig, ) from packit.config.aliases import get_build_targets +from packit.config.notifications import ( + NotificationsConfig, + FailureCommentNotificationsConfig, +) +from packit.copr_helper import CoprHelper from packit.local_project import LocalProject from packit.utils.repo import RepositoryCache from packit_service.config import ServiceConfig @@ -2786,7 +2785,7 @@ def test_repository_cache_invocation(): service_config.add_repositories_to_repository_cache = False service_config.command_handler_work_dir = "/tmp/some-dir" - copr_build_helper = CoprBuildJobHelper( + copr_build_helper = KojiBuildJobHelper( service_config=service_config, package_config=PackageConfig( jobs=[ @@ -2828,8 +2827,7 @@ def test_repository_cache_invocation(): flexmock(RepositoryCache).should_call("__init__").once() flexmock(RepositoryCache).should_receive("get_repo").with_args( - "https://github.com/some-namespace/some-repo.git", - directory=Path("/tmp/some-dir"), + "https://github.com/some-namespace/some-repo.git", directory="/tmp/some-dir" ).and_return( flexmock( git=flexmock().should_receive("checkout").and_return().mock(), diff --git a/tests/unit/test_steve.py b/tests/unit/test_steve.py index 854e79bb9..bcc06156c 100644 --- a/tests/unit/test_steve.py +++ b/tests/unit/test_steve.py @@ -18,7 +18,7 @@ from packit.api import PackitAPI from packit.config import JobConfigTriggerType from packit.distgit import DistGit -from packit.local_project import LocalProject +from packit.local_project import LocalProject, LocalProjectBuilder from packit_service.config import ServiceConfig from packit_service.constants import ( TASK_ACCEPTED, @@ -84,8 +84,8 @@ def test_process_message(event, private, enabled_private_namespaces, success): gh_project.should_receive("get_files").and_return([]) # specfile gh_project.should_receive("get_files").and_return(["packit.yaml", "setup.cfg"]) gh_project.default_branch = "main" - lp = flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: lp) lp.git_project = gh_project lp.working_dir = "" flexmock(DistGit).should_receive("local_project").and_return(lp)