Get parent class as one row to avoid warnings #5822
Conversation
There should only ever be one match. See: https://progress.opensuse.org/issues/154735
| @@ -68,4 +68,14 @@ sub query_for_settings ($self, $args) { | |||
| return $self->search({-and => \@conds}); | |||
| } | |||
|
|
|||
| sub parent_worker_class ($self, $parent_job_id, $worker_class) { | |||
| my $result = $self->search({parent_job_id => $job_id, key => 'WORKER_CLASS'})->next; | |||
There was a problem hiding this comment.
if we only want one row, it's better to tell that the database
| my $result = $self->search({parent_job_id => $job_id, key => 'WORKER_CLASS'})->next; | |
| my $result = $self->search({parent_job_id => $job_id, key => 'WORKER_CLASS'}, {rows => 1})->single; |
since there can be multiple WORKER_CLASS entries, we're still relying on the internal order of postgres here I guess.
| die | ||
| "Worker class $worker_class does not match the worker class of its directly chained parent ($parent_worker_class)" | ||
| unless $worker_class eq $parent_worker_class; | ||
| die "Ambiguous job settings for dependent jobs" if $result->next; |
There was a problem hiding this comment.
There can be multiple entries, that's valid
|
Maybe we even want to copy all WORKER_CLASS entries instead of just the first? |
|
OTOH I'm wondering why we don't see this warning more often. edit: it seems it doesn' t happen that often that a job has dependency type DIRECTLY_CHAINED (found the most recent one on osd at 2024-07-31) plus more than one WORKER_CLASS. |
| die | ||
| "Worker class $worker_class does not match the worker class of its directly chained parent ($parent_worker_class)" | ||
| unless $worker_class eq $parent_worker_class; | ||
| die "Ambiguous job settings for dependent jobs" if $result->next; |
There was a problem hiding this comment.
We may have multiple WORKER_CLASS settings per job and that's perfectly fine. This code should actually get all of them and then compare whether the full set of worker classes is identical to the full set of worker classes of the parent job (the order doesn't matter).
Note that this means that already the line $job->settings->find({key => 'WORKER_CLASS'}); is problematic. Also in this case we need to handle that there can be multiple worker classes (and then compare it to the multiple worker classes the call you're trying to fix might return).
There was a problem hiding this comment.
Right, like I wrote. And in the case of ResultSet::Jobs::create_from_settings we probably want to copy every WORKER_CLASS to the child
|
This pull request is now in conflicts. Could you fix it? 🙏 |
|
Closing in favor of #5936 |
There should only ever be one match.
See: https://progress.opensuse.org/issues/154735