freshmaker

#371 Support for event canceling
Merged by fivaldi. Opened by fivaldi.
fivaldi/freshmaker fivaldi_canceling_events  into  master

Download 371.patch
  • Frontend checks the request, cancels the database event with the corresponding builds and generates a message. The message contains builds_id of builds affected by canceling. It also has reference to the canceled event: event_id.
  • Backend parses the message (using the FreshmakerManageRequestParser) and generates FreshmakerManageEvent. The event holds the body of the original message.
  • The event is further processed by CancelEventOnFreshmakerManageRequest handler, which finds build_id for each ArtifactBuild and tries to cancel them in external build system (Koji). For builds, which failed to cancel, it generates a list of builds_id, which is inject into a new event returned from the previous/already handled one. This way several retries are performed, until max_retries is reached.

The new approach of "Freshmaker manage" messages can be reused for other similar scenarios.

UPDATE: Core of the retry machinery has moved to FreshmakerManageEvent. There's a comment in the code explaining details.

rebased onto c88f673b314858d2e7951e1039d9cfc89a5c381d

rebased onto 21109d5590a4cd34a56d808ab7eeb0eb625ebdba

pretty please pagure-ci rebuild

rebased onto 88eb923d50a473f18566fb0c0983db1830bfb253

rebased onto 50d5e0cbb9c4f5aa6c8c48103b79ad17326a6345

rebased onto 303d2a185d3d5701c4d390d7deb731ba50ac8167

rebased onto 6bef6a1f92c7356f7e3c5a74e6e2b19aa74c2a9f

rebased onto 307e9fb40f8ce142bdfafef16bf854372a85457d

pretty please pagure-ci rebuild

I know we only have single FreshmakerManageEvent, but can we force this handler to really handle only the 'cancelevent' case?

I'm thinking about adding topic to FreshmakerManageEventand checking here if the topic is reallycancelevent`.

This will prevent this handler from running in case we add another manage event.

just curious, why is this needed?

Why is this change needed?

Looks good generally, I just have few questions and one change request ;).

This is here to be able to further filter the builds, see https://pagure.io/freshmaker/pull-request/371#_7__34

See below the change in models:Event.builds. After the change, it returns AppenderQuery. The for loops are not affected by the change, but e.g. here, the AppenderQuery needs to be processed first.

As I understand, messages from message bus always go through the parser, which does exactly, what you mean. (And I also want this behavior to be there - checking the topic/action.) I am only afraid of messages generated internally, which needn't(?) go through the parser. Then we maybe want to have some check in the __new__ method/constructor of FreshmakerManageEvent?
Edited by fivaldi

If we add new action in parser, we will need to remember to also update this handler to not handle that new action. Instead, I would like to write a code in a way that we can add new action in parser and do not care about the existing handlers. It would be clear that this handler really handles just the cancelevent action and will do that even if we implement some new action in the future.

rebased onto 1b1e30901586a00b3f494759a1996d73ff432b0c

Please for merging https://pagure.io/freshmaker/pull-request/373 before this PR, so that we can check the unit tests are really OK in the CI for this PR.
Edited by fivaldi

The @jkaluza's change request was done in 1b1e309.

pretty please pagure-ci rebuild

rebased onto 4f203838f0f5ccbb051b28df57834b197b420fd3

rebased onto 4dac0148b2250ea7f6f9ceeaf257237fbe84874e

rebased onto 8bc180fe5990110540d492f3a3ff5cba34f3dcd7

Pull-Request has been merged by fivaldi
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.