Skip to content

Commit

Permalink
Utilise LocalProjectBuilder for LP initalisation (#2449)
Browse files Browse the repository at this point in the history
Utilise LocalProjectBuilder for LP initalisation

With this change, for copr builds also try to not clone the repo.
Fixes #1955
Related to #2411
For now no release notes, I would like to see whether this will work on staging as intended.
RELEASE NOTES BEGIN
N/A
RELEASE NOTES END

Reviewed-by: František Lachman <[email protected]>
  • Loading branch information
softwarefactory-project-zuul[bot] committed Jun 26, 2024
2 parents e0fc2f5 + f74362a commit aad797a
Show file tree
Hide file tree
Showing 10 changed files with 86 additions and 45 deletions.
1 change: 1 addition & 0 deletions packit_service/worker/helpers/build/koji_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
30 changes: 24 additions & 6 deletions packit_service/worker/helpers/job_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -28,6 +33,8 @@


class BaseJobHelper:
require_git_repo_in_local_project: bool = False

def __init__(
self,
service_config: ServiceConfig,
Expand Down Expand Up @@ -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,
Expand All @@ -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

Expand Down
25 changes: 19 additions & 6 deletions packit_service/worker/mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -214,24 +214,37 @@ 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,
add_new=self.service_config.add_repositories_to_repository_cache,
)
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


Expand Down
23 changes: 11 additions & 12 deletions tests/integration/test_dg_commit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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


Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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())
Expand Down
3 changes: 2 additions & 1 deletion tests/integration/test_issue_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion tests/integration/test_new_hotness_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions tests/integration/test_pr_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)


Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down
20 changes: 14 additions & 6 deletions tests/integration/test_release_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 = ""
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 = ""
Expand Down Expand Up @@ -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 = ""
Expand Down
Loading

0 comments on commit aad797a

Please sign in to comment.