Toggle modules to active=True when they are done building. #52
Conversation
pdcupdater/handlers/modules.py
Outdated
@@ -75,6 +75,12 @@ def handle(self, pdc, msg): | |||
|
|||
unreleased_variant = self.get_or_create_unreleased_variant(pdc, body) | |||
|
|||
if body['state_name'] == 'ready': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just use the state
variable here instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair.
uid = unreleased_variant['variant_uid'] | ||
# This submits an HTTP PATCH. | ||
# The '/' is necessary to avoid losing the body in a 301. | ||
pdc['unreleasedvariants'][uid + '/'] += {'variant_uid': uid, 'active': True} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering why the variant_uid
is set here. I'm assuming that the uid is already correct since you seem to be querying by it with pdc['unreleasedvariants'][uid + '/']
. I might be just misunderstanding what beanbag does in this instance though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I thought that too... but in practice PDC expects the patch data to also contain the lookup_field otherwise it considers it an incomplete partial patch (or something like that...).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh okay. Thanks for clarifying!
@@ -130,12 +136,10 @@ def get_or_create_unreleased_variant(self, pdc, body): | |||
# version/release, but for now we just do the right mapping here... | |||
variant_version = body['stream'] # This is supposed to be equal to version | |||
variant_release = body['version'] # This is supposed to be equal to release | |||
variant_uid = "%s-%s-%s" % (variant_id, variant_version, variant_release) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not important, but I like the way this is done in create_unreleased
better :) :
variant_uid = "{n}-{v}-{r}".format(n=name, v=version, r=release)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, I'll punt on this one. ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Punt? Wrong metaphor. Kick a field goal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Punt is fine 😛
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ralphbean and the sports metaphor:
+1 |
Thanks! Will wait on fedora-modularity/product-definition-center#11 to merge this one. |
+1 |
This is for https://pagure.io/fm-orchestrator/issue/407
Here we also switch to using the variant_uid to lookup individual modules in
PDC which is introduced in fedora-modularity/product-definition-center#11.