Skip to content

Commit

Permalink
feat(events): implement ‹event_type()›
Browse files Browse the repository at this point in the history
Till now we have used ‹.__name__› on the event classes for serialization
for the Celery, but after the refactor the ‹.__name__› contains just the
last name in the whole path (i.e., only the class name is included,
modules are lost) which creates conflicts and also breaks the matching
of serialized ‹event_type› to the respective classes.

Therefore implement a new abstract class method that returns the whole
event type (including the source and any additional subtyping).

Also implement the ‹event_type()› for some of the abstract classes that
are used in the tests, with a check that the tests are running to ensure
that abstract classes are not constructed during the runtime.

Signed-off-by: Matej Focko <[email protected]>
  • Loading branch information
mfocko committed Jan 22, 2025
1 parent 2170b1a commit 2fcca9a
Show file tree
Hide file tree
Showing 38 changed files with 273 additions and 101 deletions.
16 changes: 8 additions & 8 deletions packit_service/worker/checker/bodhi.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ def pre_check(self) -> bool:
"""

if self.data.event_type in (
PullRequestCommentPagureEvent.__name__,
KojiBuildEvent.__name__,
KojiBuildTagEvent.__name__,
PullRequestCommentPagureEvent.event_type(),
KojiBuildEvent.event_type(),
KojiBuildTagEvent.event_type(),
):
for koji_build_data in self:
if koji_build_data.state != KojiBuildState.complete:
Expand Down Expand Up @@ -75,7 +75,7 @@ class IsKojiBuildOwnerMatchingConfiguration(Checker, GetKojiBuildEventMixin):
def pre_check(self) -> bool:
"""Check if the build submitter matches the configuration"""

if self.data.event_type in (KojiBuildEvent.__name__,):
if self.data.event_type in (KojiBuildEvent.event_type(),):
owner = self.koji_build_event.owner
configured_builders = self.job_config.allowed_builders

Expand Down Expand Up @@ -120,8 +120,8 @@ class HasIssueCommenterRetriggeringPermissions(ActorChecker, ConfigFromEventMixi
def _pre_check(self) -> bool:
has_write_access = self.project.has_write_access(user=self.actor)
if self.data.event_type in (
IssueCommentEvent.__name__,
IssueCommentGitlabEvent.__name__,
IssueCommentEvent.event_type(),
IssueCommentGitlabEvent.event_type(),
):
logger.debug(
f"Re-triggering Bodhi update through comment in "
Expand All @@ -147,7 +147,7 @@ def _pre_check(self) -> bool:
return False

return True
if self.data.event_type in (PullRequestCommentPagureEvent.__name__,):
if self.data.event_type in (PullRequestCommentPagureEvent.event_type(),):
logger.debug(
f"Re-triggering Bodhi update via dist-git comment in "
f"repo {self.project_url} and #PR {self.data.pr_id} "
Expand Down Expand Up @@ -179,7 +179,7 @@ def _pre_check(self) -> bool:
class IsAuthorAPackager(ActorChecker, PackitAPIWithDownstreamMixin):
def _pre_check(self) -> bool:
if self.data.event_type not in (
PullRequestCommentPagureEvent.__name__,
PullRequestCommentPagureEvent.event_type(),
) or self.is_packager(user=self.actor):
return True

Expand Down
2 changes: 1 addition & 1 deletion packit_service/worker/checker/copr.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def pre_check(
self,
) -> bool:
if (
self.data.event_type == MergeRequestGitlabEvent.__name__
self.data.event_type == MergeRequestGitlabEvent.event_type()
and self.data.event_dict["action"] == GitlabEventAction.closed.value
):
# Not interested in closed merge requests
Expand Down
20 changes: 10 additions & 10 deletions packit_service/worker/checker/distgit.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

class LabelsOnDistgitPR(Checker, GetPagurePullRequestMixin):
def pre_check(self) -> bool:
if self.data.event_type not in (PushPagureEvent.__name__,) or not (
if self.data.event_type not in (PushPagureEvent.event_type(),) or not (
self.job_config.require.label.present or self.job_config.require.label.absent
):
return True
Expand All @@ -47,8 +47,8 @@ def pre_check(self) -> bool:
class PermissionOnDistgit(Checker, GetPagurePullRequestMixin):
def pre_check(self) -> bool:
if self.data.event_type in (
PushPagureEvent.__name__,
KojiBuildTagEvent.__name__,
PushPagureEvent.event_type(),
KojiBuildTagEvent.event_type(),
) and self.data.git_ref not in (
configured_branches := get_branches(
*self.job_config.dist_git_branches,
Expand All @@ -62,7 +62,7 @@ def pre_check(self) -> bool:
)
return False

if self.data.event_type in (PushPagureEvent.__name__,):
if self.data.event_type in (PushPagureEvent.event_type(),):
if self.pull_request:
pr_author = self.get_pr_author()
logger.debug(f"PR author: {pr_author}")
Expand Down Expand Up @@ -94,7 +94,7 @@ def pre_check(self) -> bool:
f"configuration: {self.job_config.allowed_committers}.",
)
return False
elif self.data.event_type in (PullRequestCommentPagureEvent.__name__,):
elif self.data.event_type in (PullRequestCommentPagureEvent.event_type(),):
comment = self.data.event_dict.get("comment", "")
commands = get_packit_commands_from_comment(
comment,
Expand Down Expand Up @@ -136,8 +136,8 @@ class HasIssueCommenterRetriggeringPermissions(ActorChecker):

def _pre_check(self) -> bool:
if self.data.event_type in (
IssueCommentEvent.__name__,
IssueCommentGitlabEvent.__name__,
IssueCommentEvent.event_type(),
IssueCommentGitlabEvent.event_type(),
):
logger.debug(
f"Re-triggering downstream koji-build through comment in "
Expand Down Expand Up @@ -181,7 +181,7 @@ class TaggedBuildIsNotABuildOfSelf(Checker):
"""

def pre_check(self) -> bool:
if self.data.event_type in (KojiBuildTagEvent.__name__,) and (
if self.data.event_type in (KojiBuildTagEvent.event_type(),) and (
self.data.event_dict.get("package_name") == self.job_config.downstream_package_name
):
logger.info("Skipping build triggered by tagging a build of self.")
Expand Down Expand Up @@ -215,12 +215,12 @@ def pre_check(self) -> bool:
valid = False

if self.package_config.upstream_project_url and (
self.data.event_type in (NewHotnessUpdateEvent.__name__,) and not self.data.tag_name
self.data.event_type in (NewHotnessUpdateEvent.event_type(),) and not self.data.tag_name
):
msg_to_report = "We were not able to get the upstream tag name."
valid = False

if self.data.event_type in (PullRequestCommentPagureEvent.__name__,):
if self.data.event_type in (PullRequestCommentPagureEvent.event_type(),):
commenter = self.data.actor
logger.debug(
f"Triggering pull-from-upstream through comment by: {commenter}",
Expand Down
6 changes: 3 additions & 3 deletions packit_service/worker/checker/koji.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ def pre_check(self) -> bool:
class PermissionOnKoji(Checker, GetKojiBuildJobHelperMixin):
def pre_check(self) -> bool:
if (
self.data.event_type == MergeRequestGitlabEvent.__name__
self.data.event_type == MergeRequestGitlabEvent.event_type()
and self.data.event_dict["action"] == GitlabEventAction.closed.value
):
# Not interested in closed merge requests
return False

if self.data.event_type in (
PullRequestGithubEvent.__name__,
MergeRequestGitlabEvent.__name__,
PullRequestGithubEvent.event_type(),
MergeRequestGitlabEvent.event_type(),
):
user_can_merge_pr = self.project.can_merge_pr(self.data.actor)
if not (user_can_merge_pr or self.data.actor in self.service_config.admins):
Expand Down
16 changes: 16 additions & 0 deletions packit_service/worker/events/abstract/comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
abstract-comment event classes.
"""

import os
import re
from logging import getLogger
from typing import Optional, Union
Expand Down Expand Up @@ -80,6 +81,11 @@ def __init__(
self._build_targets_override = build_targets_override
self._tests_targets_override = tests_targets_override

@classmethod
def event_type(cls) -> str:
assert os.environ.get("PYTEST_VERSION"), "Should be initialized only during tests"
return "test.abstract.comment.PullRequest"

@property
def commit_sha(self) -> str: # type:ignore
# mypy does not like properties
Expand Down Expand Up @@ -165,6 +171,11 @@ def __init__(
self._comment_object = comment_object
self._issue_object: Optional[OgrIssue] = None

@classmethod
def event_type(cls) -> str:
assert os.environ.get("PYTEST_VERSION"), "Should be initialized only during tests"
return "test.abstract.comment.Issue"

@property
def tag_name(self):
if not self._tag_name:
Expand Down Expand Up @@ -227,6 +238,11 @@ def __init__(
self._tag_name: Optional[str] = None
self._branch: Optional[str] = None

@classmethod
def event_type(cls) -> str:
assert os.environ.get("PYTEST_VERSION"), "Should be initialized only during tests"
return "test.abstract.comment.Commit"

@property
def identifier(self) -> Optional[str]:
return self.tag_name or self.branch
Expand Down
8 changes: 8 additions & 0 deletions packit_service/worker/events/anitya/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ def __init__(
self._version = version
self.bug_id = bug_id

@classmethod
def event_type(cls) -> str:
return "anitya.NewHotness"

@property
def version(self) -> str:
return self._version
Expand All @@ -61,6 +65,10 @@ def __init__(

self._versions = versions

@classmethod
def event_type(cls) -> str:
return "anitya.VersionUpdate"

@property
def version(self) -> Optional[str]:
# we will decide the version just when syncing release
Expand Down
14 changes: 12 additions & 2 deletions packit_service/worker/events/copr.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Copyright Contributors to the Packit project.
# SPDX-License-Identifier: MIT

import os
from logging import getLogger
from typing import Optional, Union

Expand Down Expand Up @@ -67,6 +68,11 @@ def __init__(
self.pkg = pkg
self.timestamp = timestamp

@classmethod
def event_type(cls) -> str:
assert os.environ.get("PYTEST_VERSION"), "Should be initialized only during tests"
return "test.copr.Build"

def get_db_project_object(self) -> Optional[AbstractProjectObjectDbType]:
return self.build.get_project_event_object()

Expand Down Expand Up @@ -169,8 +175,12 @@ def get_copr_build_logs_url(self) -> str:


class Start(CoprBuild):
pass
@classmethod
def event_type(cls) -> str:
return "copr.build.Start"


class End(CoprBuild):
pass
@classmethod
def event_type(cls) -> str:
return "copr.build.End"
55 changes: 32 additions & 23 deletions packit_service/worker/events/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,14 @@ def _add_project_object_and_event(self):
# TODO, do a better job
# Probably, try to recreate original classes.
if self.event_type in {
"PullRequestGithubEvent",
"PullRequestPagureEvent",
"MergeRequestGitlabEvent",
"PullRequestCommentGithubEvent",
"MergeRequestCommentGitlabEvent",
"PullRequestCommentPagureEvent",
"PullRequestFlagPagureEvent",
"CheckRerunPullRequestEvent",
"github.pr.Synchronize",
"pagure.pr.Synchronize",
"gitlab.mr.Synchronize",
"github.pr.Comment",
"pagure.pr.Comment",
"gitlab.mr.Comment",
"pagure.pr.Flag",
"github.check.PullRequest",
}:
(
self._db_project_object,
Expand All @@ -195,10 +195,10 @@ def _add_project_object_and_event(self):
commit_sha=self.commit_sha,
)
elif self.event_type in {
"PushGitHubEvent",
"PushGitlabEvent",
"PushPagureEvent",
"CheckRerunCommitEvent",
"github.push.Push",
"gitlab.push.Push",
"pagure.push.Push",
"github.check.Push",
}:
(
self._db_project_object,
Expand All @@ -212,9 +212,9 @@ def _add_project_object_and_event(self):
)

elif self.event_type in {
"ReleaseEvent",
"ReleaseGitlabEvent",
"CheckRerunReleaseEvent",
"github.release.Release",
"gitlab.release.Release",
"github.check.Release",
}:
(
self._db_project_object,
Expand All @@ -227,7 +227,7 @@ def _add_project_object_and_event(self):
commit_hash=self.commit_sha,
)
elif self.event_type in {
"NewHotnessUpdateEvent",
"anitya.NewHotness",
}:
if not self.project_url:
(
Expand Down Expand Up @@ -259,8 +259,8 @@ def _add_project_object_and_event(self):
commit_hash=self.commit_sha,
)
elif self.event_type in {
"IssueCommentEvent",
"IssueCommentGitlabEvent",
"github.issue.Comment",
"gitlab.issue.Comment",
}:
(
self._db_project_object,
Expand All @@ -272,7 +272,7 @@ def _add_project_object_and_event(self):
project_url=self.project_url,
)
elif self.event_type in {
"KojiBuildTagEvent",
"koji.base.Tag",
}:
(
self._db_project_object,
Expand All @@ -285,7 +285,7 @@ def _add_project_object_and_event(self):
project_url=self.project_url,
)
elif self.event_type in {
"KojiBuildEvent",
"koji.base.Build",
}:
(
self._db_project_object,
Expand All @@ -298,8 +298,8 @@ def _add_project_object_and_event(self):
commit_sha=self.event_dict.get("commit_sha"),
)
elif self.event_type in {
"CommitCommentGithubEvent",
"CommitCommentGitlabEvent",
"github.commit.Comment",
"gitlab.commit.Comment",
}:
if self.tag_name:
(
Expand Down Expand Up @@ -392,6 +392,14 @@ def __init__(self, created_at: Optional[Union[int, float, str]] = None):
self._db_project_object: Optional[AbstractProjectObjectDbType] = None
self._db_project_event: Optional[ProjectEventModel] = None

@classmethod
@abstractmethod
def event_type(cls) -> str:
"""
‹TODO›
"""
...

@staticmethod
def make_serializable(d: dict, skip: list) -> dict:
"""We need a JSON serializable dict (because of redis and celery tasks)
Expand Down Expand Up @@ -434,7 +442,8 @@ def get_dict(self, default_dict: Optional[dict] = None) -> dict:
d = default_dict or self.__dict__
# whole dict has to be JSON serializable because of redis
d = self.make_serializable(d, self.get_non_serializable_attributes())
d["event_type"] = self.__class__.__name__
# [TODO] check correctness, removed ‹__class__›
d["event_type"] = self.__class__.event_type()

# we are trying to be lazy => don't touch database if it is not needed
d["event_id"] = self._db_project_object.id if self._db_project_object else None
Expand Down
Loading

0 comments on commit 2fcca9a

Please sign in to comment.