freshmaker

#161 Define the BaseHandler.allow_build criteria in the BaseEvent subclasses to keep all of them consistent and in the same place
Closed by jkaluza. Opened by jkaluza.
jkaluza/freshmaker allow-build-events  into  master

Download 161.patch

This allows us to do

if not self.allow_build_by_event(ArtifactType.IMAGE, event):

instead of

         if not event.manual and not self.allow_build(
                 ArtifactType.IMAGE,
                 advisory_name=event.errata_name,
                 advisory_security_impact=event.security_impact):

and be sure we always get the right criteria with right key names everytime and in case we add new key name later, we just do that in single place in events.py.

rebased onto 67ff750cceb0950bd5a2c50818bbb1d91bbeb4d7

Code looks good for me. But, to be honest, this change makes code not easy to understand and not straightforward than current implementation from my point of view. Each time when I read the code

allow_build(
    ArtifactType.IMAGE,
    advisory_name=event.errata_name,
    advisory_security_impact=event.security_impact)

I can easily know freshmaker will check advisory_name and/or advisory_security_impact to allow or not allow current build, without switching between more API calls (there is only one call to allow_build), remembering where to define the allow build criteria and how allow_build_by_event handles to extra criteria.

In addition, it would be useful to define some convenient helper functions of allow_build, e.g. allow_image_build and allow_module_build. With those functions, we can just pass criteria, for example,

allow_image_build(
    advisory_name=event.errata_name,
    advisory_security_impact=event.security_impact)
allow_module_build( ... )

This change looks stuck. @jkaluza, should it be dropped?

Pull-Request has been closed by jkaluza
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.