From 0c4d985b69a2c6fa29dcada60907a4eaf5eeff58 Mon Sep 17 00:00:00 2001 From: Michael Tremer Date: Thu, 27 Mar 2025 11:47:29 +0000 Subject: [PATCH] release monitoring: Fix controls Signed-off-by: Michael Tremer --- src/buildservice/releasemonitoring.py | 128 ++++++++++++-------------- src/templates/monitorings/delete.html | 12 +-- src/templates/monitorings/edit.html | 50 ++++++---- src/templates/monitorings/show.html | 7 +- src/web/monitorings.py | 66 ++++++------- 5 files changed, 135 insertions(+), 128 deletions(-) diff --git a/src/buildservice/releasemonitoring.py b/src/buildservice/releasemonitoring.py index 27a1c9a8..221037b5 100644 --- a/src/buildservice/releasemonitoring.py +++ b/src/buildservice/releasemonitoring.py @@ -471,30 +471,19 @@ class Monitorings(base.Object): async def create(self, distro, name, created_by, project_id, follow="stable", create_builds=True): - monitoring = await self._get_monitoring(""" - INSERT INTO - release_monitorings - ( - distro_id, - name, - created_by, - project_id, - follow, - create_builds - ) - VALUES( - %s, %s, %s, %s, %s, %s - ) - RETURNING - * - """, distro, name, created_by, project_id, follow, create_builds, - - # Populate cache - distro=distro, + """ + Creates a new monitoring + """ + return await self.db.insert( + Monitoring, + distro = distro, + name = name, + created_by = created_by, + project_id = project_id, + follow = follow, + create_builds = create_builds, ) - return monitoring - async def search(self, name): """ Returns a bunch of packages that match the given name @@ -508,36 +497,39 @@ class Monitorings(base.Object): ) # Return all packages - return [database.Row(item) for item in response.get("items")] + return [item for item in response.get("items")] async def check(self, limit=None): """ Perform checks on monitorings """ # Fetch all monitorings that were never checked or checked longer than 24 hours ago - monitorings = self._get_monitorings(""" - SELECT - * - FROM - release_monitorings - WHERE - deleted_at IS NULL - AND - ( - last_check_at IS NULL - OR - last_check_at <= CURRENT_TIMESTAMP - INTERVAL '24 hours' - ) - ORDER BY - last_check_at ASC NULLS FIRST - LIMIT - %s - """, limit, + stmt = ( + sqlalchemy + .select( + Monitoring, + ) + .where( + Monitoring.deleted_at == None, + + sqlalchemy.or_( + Monitoring.last_check_at == None, + Monitoring.last_check_at == + sqlalchemy.func.current_timestamp() - datetime.timedelta(days=24), + ), + ) + .order_by( + Monitoring.last_check_at.asc(), + ) ) - async for monitoring in monitorings: + # Run checks for all of them + async for monitoring in self.db.fetch(stmt): await monitoring.check() + # Commit after each check + await self.db.commit() + class Monitoring(database.Base, database.BackendMixin, database.SoftDeleteMixin): __tablename__ = "release_monitorings" @@ -559,7 +551,7 @@ class Monitoring(database.Base, database.BackendMixin, database.SoftDeleteMixin) # Distro - distro = sqlalchemy.orm.relationship("Distro", lazy="joined") + distro = sqlalchemy.orm.relationship("Distro", lazy="joined", innerjoin=True) # Name @@ -577,7 +569,7 @@ class Monitoring(database.Base, database.BackendMixin, database.SoftDeleteMixin) # Created By created_by = sqlalchemy.orm.relationship( - "User", foreign_keys=[created_by_id], lazy="joined", + "User", foreign_keys=[created_by_id], lazy="joined", innerjoin=True, ) # Deleted By ID @@ -587,7 +579,7 @@ class Monitoring(database.Base, database.BackendMixin, database.SoftDeleteMixin) # Deleted By deleted_by = sqlalchemy.orm.relationship( - "User", foreign_keys=[deleted_by_id], lazy="joined", + "User", foreign_keys=[deleted_by_id], lazy="selectin", ) # Project ID @@ -618,16 +610,15 @@ class Monitoring(database.Base, database.BackendMixin, database.SoftDeleteMixin) # Delete - async def delete(self, user=None): - # Mark as deleted - await self._set_attribute_now("deleted_at") - if user: - await self._set_attribute("deleted_by", user) + async def delete(self, *args, **kwargs): + """ + Deletes this monitoring object + """ + await super().delete(*args, **kwargs) # Delete all releases - async with asyncio.TaskGroup() as tasks: - for release in self.releases: - tasks.create_task(release.delete()) + async for release in self.get_releases(): + await release.delete() # Check @@ -651,11 +642,11 @@ class Monitoring(database.Base, database.BackendMixin, database.SoftDeleteMixin) try: if self.follow == "latest": - release = await self._follow_latest(versions) + release = await self._follow_latest(versions, build=latest_build) elif self.follow == "stable": - release = await self._follow_stable(versions) + release = await self._follow_stable(versions, build=latest_build) elif self.follow == "current-branch": - release = await self._follow_current_branch(versions, latest_build) + release = await self._follow_current_branch(versions, build=latest_build) else: raise ValueError("Cannot handle follow: %s" % self.follow) @@ -676,20 +667,17 @@ class Monitoring(database.Base, database.BackendMixin, database.SoftDeleteMixin) """ # Wait until we are allowed to send an API request async with ratelimiter: - response = await self.backend.monitorings._request( + return await self.backend.monitorings._request( "GET", "/api/v2/versions/", { "project_id" : self.project_id, }, ) - # Parse the response as JSON and return it - return database.Row(response) - async def _follow_stable(self, versions, *, build): """ This will follow "stable" i.e. the latest stable version """ - for version in versions.stable_versions: + for version in versions.get("stable_versions", []): return await self.create_release(version, build=build) async def _follow_latest(self, versions, * build): @@ -706,9 +694,11 @@ class Monitoring(database.Base, database.BackendMixin, database.SoftDeleteMixin) if not build: return + # Fetch all stable versions + stable_versions = versions.get("stable_versions", []) + # Find the next version - next_version = self._find_next_version( - latest_build.pkg.evr, versions.stable_versions) + next_version = self._find_next_version(latest_build.pkg.evr, stable_versions) # Create a new release with the next version if next_version: @@ -785,7 +775,7 @@ class Monitoring(database.Base, database.BackendMixin, database.SoftDeleteMixin) """ Returns True if this version already exists """ - return version in [release.version async for release in self.releases] + return version in [release.version async for release in self.get_releases()] async def create_release(self, version, *, build): """ @@ -798,7 +788,7 @@ class Monitoring(database.Base, database.BackendMixin, database.SoftDeleteMixin) raise ReleaseExistsError(version) # Raise an error if we already have a newer build - elif self._build_exists(version): + elif await self._build_exists(version): raise BuildExistsError(version) log.info("%s: Creating new release %s" % (self, version)) @@ -825,16 +815,18 @@ class Monitoring(database.Base, database.BackendMixin, database.SoftDeleteMixin) # Builds - def _build_exists(self, version): + async def _build_exists(self, version): """ Returns True if a build with this version already exists """ + latest_build = await self.get_latest_build() + # If there is no build to check against we return False - if not self.latest_build: + if not latest_build: return False # Compare the versions - if pakfire.version_compare(self.latest_build.pkg.evr, version) > 0: + if pakfire.version_compare(latest_build.pkg.evr, version) > 0: return True return False diff --git a/src/templates/monitorings/delete.html b/src/templates/monitorings/delete.html index d8b367fe..a4d9f89e 100644 --- a/src/templates/monitorings/delete.html +++ b/src/templates/monitorings/delete.html @@ -1,6 +1,6 @@ -{% extends "../modal.html" %} +{% extends "modal.html" %} -{% block title %}{{ _("Delete Release Monitoring") }} - {{ monitoring }}{% end block %} +{% block title %}{{ _("Delete Release Monitoring") }} - {{ monitoring }}{% endblock %} {% block breadcrumbs %} -{% end block %} +{% endblock %} {% block modal_title %}

{{ _("Delete Release Monitoring") }}

{{ monitoring }}
-{% end block %} +{% endblock %} {% block modal %}
- {% raw xsrf_form_html() %} + {{ xsrf_form_html() | safe }}

@@ -44,4 +44,4 @@

-{% end block %} +{% endblock %} diff --git a/src/templates/monitorings/edit.html b/src/templates/monitorings/edit.html index 9690144b..9bfbd0b3 100644 --- a/src/templates/monitorings/edit.html +++ b/src/templates/monitorings/edit.html @@ -1,12 +1,12 @@ -{% extends "../modal.html" %} +{% extends "modal.html" %} {% block title %} {% if monitoring %} {{ _("Edit Release Monitoring") }} - {{ monitoring }} {% else %} {{ _("Create Release Monitoring") }} - {% end %} -{% end block %} + {% endif %} +{% endblock %} {% block breadcrumbs %} -{% end block %} +{% endblock %} {% block modal_title %} {% if monitoring %} @@ -43,30 +43,33 @@
{{ monitoring }}
{% else %}

{{ _("Create A New Release Monitoring") }}

- {% end %} -{% end block %} + {% endif %} +{% endblock %} {% block modal %}
- {% raw xsrf_form_html() %} + {{ xsrf_form_html() | safe }} {# Project & Follow can only be set once #} {% if not monitoring %} {# Project #}
+
-
+
+

{{ _("Select an upstream project that matches this package.") }}

@@ -80,32 +83,39 @@ {# Follow #}
- + +
-
+
+

{{ _("Select which releases to follow.") }}

- {% end %} + {% endif %} {# Create Builds? #}
- + +
@@ -121,7 +131,7 @@ - {% end %} + {% endif %}
-{% end block %} +{% endblock %} diff --git a/src/templates/monitorings/show.html b/src/templates/monitorings/show.html index 8d4c3b17..817d6cd5 100644 --- a/src/templates/monitorings/show.html +++ b/src/templates/monitorings/show.html @@ -67,13 +67,14 @@
{% endif %} - {% if monitoring.latest_build %} + {% set latest_build = monitoring.get_latest_build() %} + {% if latest_build %}
diff --git a/src/web/monitorings.py b/src/web/monitorings.py index 4bbdc358..9f5a141b 100644 --- a/src/web/monitorings.py +++ b/src/web/monitorings.py @@ -41,12 +41,12 @@ class CreateHandler(base.BaseHandler): @base.authenticated async def get(self, slug, name): # Fetch the distribution - distro = self.backend.distros.get_by_slug(slug) + distro = await self.backend.distros.get_by_slug(slug) if not distro: raise tornado.web.HTTPError(404, "Could not find distro %s" % slug) # Fetch the monitoring - monitoring = self.backend.monitorings.get_by_distro_and_name(distro, name) + monitoring = await self.backend.monitorings.get_by_distro_and_name(distro, name) if monitoring: raise tornado.web.HTTPError(400, "Monitoring for %s in %s already exists" % (name, distro)) @@ -60,18 +60,18 @@ class CreateHandler(base.BaseHandler): # Search for projects projects = await self.backend.monitorings.search(q) - self.render("monitorings/edit.html", monitoring=None, distro=distro, name=name, - projects=projects) + await self.render("monitorings/edit.html", monitoring=None, + distro=distro, name=name, projects=projects) @base.authenticated async def post(self, slug, name): # Fetch the distribution - distro = self.backend.distros.get_by_slug(slug) + distro = await self.backend.distros.get_by_slug(slug) if not distro: raise tornado.web.HTTPError(404, "Could not find distro %s" % slug) # Fetch the monitoring - monitoring = self.backend.monitorings.get_by_distro_and_name(distro, name) + monitoring = await self.backend.monitorings.get_by_distro_and_name(distro, name) if monitoring: raise tornado.web.HTTPError(400, "Monitoring for %s in %s already exists" % (name, distro)) @@ -84,19 +84,23 @@ class CreateHandler(base.BaseHandler): follow = self.get_argument("follow") create_builds = self.get_argument_bool("create_builds") - with self.db.transaction(): + async with await self.db.transaction(): try: # Create a new monitoring - monitoring = await self.backend.monitorings.create(distro, name, - created_by=self.current_user, project_id=project_id, - follow=follow, create_builds=create_builds, + monitoring = await self.backend.monitorings.create( + distro = distro, + name = name, + created_by = self.current_user, + project_id = project_id, + follow = follow, + create_builds = create_builds, ) except ValueError as e: raise tornado.web.HTTPError(400, "%s" % e) from e # Perform check immediately - with self.db.transaction(): + async with await self.db.transaction(): await monitoring.check() # Redirect to the newly created monitoring @@ -105,14 +109,14 @@ class CreateHandler(base.BaseHandler): class EditHandler(base.BaseHandler): @base.authenticated - def get(self, slug, name): + async def get(self, slug, name): # Fetch the distribution - distro = self.backend.distros.get_by_slug(slug) + distro = await self.backend.distros.get_by_slug(slug) if not distro: raise tornado.web.HTTPError(404, "Could not find distro %s" % slug) # Fetch the monitoring - monitoring = self.backend.monitorings.get_by_distro_and_name(distro, name) + monitoring = await self.backend.monitorings.get_by_distro_and_name(distro, name) if not monitoring: raise tornado.web.HTTPError(404, "Could not find monitoring for %s in %s" % (name, distro)) @@ -120,17 +124,17 @@ class EditHandler(base.BaseHandler): if not monitoring.has_perm(self.current_user): raise tornado.web.HTTPError(403) - self.render("monitorings/edit.html", monitoring=monitoring, distro=distro, name=name) + await self.render("monitorings/edit.html", monitoring=monitoring, distro=distro, name=name) @base.authenticated - def post(self, slug, name): + async def post(self, slug, name): # Fetch the distribution - distro = self.backend.distros.get_by_slug(slug) + distro = await self.backend.distros.get_by_slug(slug) if not distro: raise tornado.web.HTTPError(404, "Could not find distro %s" % slug) # Fetch the monitoring - monitoring = self.backend.monitorings.get_by_distro_and_name(distro, name) + monitoring = await self.backend.monitorings.get_by_distro_and_name(distro, name) if not monitoring: raise tornado.web.HTTPError(404, "Could not find monitoring for %s in %s" % (name, distro)) @@ -138,22 +142,22 @@ class EditHandler(base.BaseHandler): if not monitoring.has_perm(self.current_user): raise tornado.web.HTTPError(403) - with self.db.transaction(): - monitoring.create_builds = self.get_argument_bool("create_builds") + # Store values + monitoring.create_builds = self.get_argument_bool("create_builds") self.redirect(monitoring.url) class DeleteHandler(base.BaseHandler): @base.authenticated - def get(self, slug, name): + async def get(self, slug, name): # Fetch the distribution - distro = self.backend.distros.get_by_slug(slug) + distro = await self.backend.distros.get_by_slug(slug) if not distro: raise tornado.web.HTTPError(404, "Could not find distro %s" % slug) # Fetch the monitoring - monitoring = self.backend.monitorings.get_by_distro_and_name(distro, name) + monitoring = await self.backend.monitorings.get_by_distro_and_name(distro, name) if not monitoring: raise tornado.web.HTTPError(404, "Could not find monitoring for %s in %s" % (name, distro)) @@ -161,17 +165,17 @@ class DeleteHandler(base.BaseHandler): if not monitoring.has_perm(self.current_user): raise tornado.web.HTTPError(403) - self.render("monitorings/delete.html", monitoring=monitoring) + await self.render("monitorings/delete.html", monitoring=monitoring) @base.authenticated async def post(self, slug, name): # Fetch the distribution - distro = self.backend.distros.get_by_slug(slug) + distro = await self.backend.distros.get_by_slug(slug) if not distro: raise tornado.web.HTTPError(404, "Could not find distro %s" % slug) # Fetch the monitoring - monitoring = self.backend.monitorings.get_by_distro_and_name(distro, name) + monitoring = await self.backend.monitorings.get_by_distro_and_name(distro, name) if not monitoring: raise tornado.web.HTTPError(404, "Could not find monitoring for %s in %s" % (name, distro)) @@ -179,8 +183,8 @@ class DeleteHandler(base.BaseHandler): if not monitoring.has_perm(self.current_user): raise tornado.web.HTTPError(403) - with self.db.transaction(): - await monitoring.delete(self.current_user) + # Delete the monitoring + await monitoring.delete(self.current_user) self.redirect("/packages/%s" % monitoring.name) @@ -189,12 +193,12 @@ class CheckHandler(base.BaseHandler): @base.authenticated async def post(self, slug, name): # Fetch the distribution - distro = self.backend.distros.get_by_slug(slug) + distro = await self.backend.distros.get_by_slug(slug) if not distro: raise tornado.web.HTTPError(404, "Could not find distro %s" % slug) # Fetch the monitoring - monitoring = self.backend.monitorings.get_by_distro_and_name(distro, name) + monitoring = await self.backend.monitorings.get_by_distro_and_name(distro, name) if not monitoring: raise tornado.web.HTTPError(404, "Could not find monitoring for %s in %s" % (name, distro)) @@ -202,7 +206,7 @@ class CheckHandler(base.BaseHandler): if not monitoring.has_perm(self.current_user): raise tornado.web.HTTPError(403) - # Perform check (it is already starting its own transaction) + # Perform check await monitoring.check() # Redirect back -- 2.47.2