#29 Added AMQP fedora messaging & changed project structure
Merged by dcavalca. Opened by oidoming.
centos-sig-hyperscale/ oidoming/package-updates main  into  main

Download 29.patch

Changes:

  • Added fedora messaging to listen on AMQP cbs/koji notifications for issue #114 (auto close issues)

  • Refactor project structure to make it run in MQTT and AMQP modes in separated instances (MQTT and AMQP directories).

  • Now issues will contain a new tag: “package_name.centos_version”. This is to facilitate the filtering of a package for a specific centos version when auto closing an issue. E.g., if we have issues for same package but different centos versions.
    pagure.io API doesn’t have a way to filter issues that contains only both specified tags, for example, if I want to get an issue for dnf and c8s version it will be returned all issues that have either dnf tag or c8s tag and not only issues that contains both tags.

  • Changed c8s for c9s as the base image in the Dockerfile

  • Project constants are in a separated file (constants.py)

  • This PR also solves issue #124: Changed SQLite functions and encapsulated into classes to avoid passing db connection on each function/method as parameter. Also implemented sqslite3.Row for better readability, now accessing values with name = package['name'] instead of name = package[1]

Note: Setup and MQTT logic keeps the same, just moved to different directory.

Diagram of the logic for auto closing issues, describes logic under the AMQP directory: https://pagure.io/fork/oidoming/centos-sig-hyperscale/package-updates/raw/main/f/images/DiagramFlowAutoClose.png

1 new commit added

  • Fix dockerfile user permissions & docstrings

AMQP and MQTT modes now working in openshift

Thanks! QQ, I see https://pagure.io/centos-sig-hyperscale/package-updates/issues has a couple of dups, namely
https://pagure.io/centos-sig-hyperscale/package-updates/issue/33
and
https://pagure.io/centos-sig-hyperscale/package-updates/issue/32

The former was opened by a human (@fanjunkong) and is missing rpm.c8s tag, while the latter was the bot and has the tag rpm.c8s.

Is this a side effect of testing? or a side effect of defaults in the code base that cause anyone trying the code to populate issues here?

On this PR I added this new "rpm.c8s" tag (package_name.centos_version) into the code so the bot now adds the new tag to filter same package of different centos versions when auto closing an issue (due to pagure limitations)

I can see @fanjunkong has forked the project and probably is running the previous (current?) version, this is why the rpm.c8s tag is missing when he creates an issue

I can see @fanjunkong has forked the project and probably is running the previous (current?) version, this is why the rpm.c8s tag is missing when he creates an issue

Maybe we should have defaults that are in such a way that people running the fork don't populate stuff in this repo unless they explicitely provide the right parameters?

Yeah definitely, https://pagure.io/centos-sig-hyperscale/sig/issue/122 should solve this by making the repo URL configurable, I'm working on it and I will add it to a different PR, this one is already too large :stuck_out_tongue_closed_eyes:

is this left over? If you intend to log it, maybe use the logging module instead.

is there any reason the UID/GID is used instead of username/group name?

is this left over? If you intend to log it, maybe use the logging module instead.

yeah the logging module is better, I need to replace the print statements all over the project with the logging module as well, we already have a ticket https://pagure.io/centos-sig-hyperscale/sig/issue/123. Same case as the ticket mentioned before (WIP)

is there any reason the UID/GID is used instead of username/group name?

To prevent privileged issues, OpenShift by default assigns a random UID that is part of the 0 GID when runs a container, so doesn’t matter if a username is specified within the Dockerfile, it will run with this random user.
https://developers.redhat.com/blog/2020/10/26/adapting-docker-and-kubernetes-containers-to-run-on-red-hat-openshift-container-platform#group_ownership_and_file_permission

typo: received

nit: return self.build_status == 1 ?

The documentation is not reflecting what the function does.

The documentation is not reflecting what the function does.

Sorry for the feedback provided in piece meal format :) I find it difficult to review code through pagure as I can only comment at the whole change level, but not at the commit level. This is making it particularly difficult to comment on independent chunks of code you did.

1 new commit added

  • Change unclear documentation/code

Thanks for the feedback, if there is nothing else then we are good to merge :)

Pull-Request has been merged by dcavalca

Metadata