From 6189ba9c5e5308e17a7d1fb7f38443272a70f072 Mon Sep 17 00:00:00 2001 From: Pierre Bourdon Date: Fri, 12 Apr 2024 17:33:27 +0200 Subject: [PATCH] web: replace 'errormsg' with 'errormsg IS NULL' in most cases This is implement in an extremely hacky way due to poor DBIx feature support. Ideally, what we'd need is a way to tell DBIx to ignore the errormsg column unless explicitly requested, and to automatically add a computed 'errormsg IS NULL' column in others. Since it does not support that, this commit instead hacks some support via method overrides while taking care to not break anything obvious. --- package.nix | 1 + src/lib/Hydra/Controller/Jobset.pm | 6 ++++ src/lib/Hydra/Controller/JobsetEval.pm | 2 ++ src/lib/Hydra/Helper/Nix.pm | 3 +- .../Hydra/Schema/Result/EvaluationErrors.pm | 2 ++ src/lib/Hydra/Schema/Result/Jobsets.pm | 2 ++ .../Schema/ResultSet/EvaluationErrors.pm | 30 +++++++++++++++++++ src/lib/Hydra/Schema/ResultSet/Jobsets.pm | 30 +++++++++++++++++++ src/root/common.tt | 4 +-- src/root/jobset-eval.tt | 4 +-- src/root/jobset.tt | 6 ++-- t/evaluator/evaluate-constituents-broken.t | 2 +- t/lib/CliRunners.pm | 4 +-- t/queue-runner/direct-indirect-constituents.t | 2 +- 14 files changed, 85 insertions(+), 13 deletions(-) create mode 100644 src/lib/Hydra/Schema/ResultSet/EvaluationErrors.pm create mode 100644 src/lib/Hydra/Schema/ResultSet/Jobsets.pm diff --git a/package.nix b/package.nix index f8b1849f..623bebeb 100644 --- a/package.nix +++ b/package.nix @@ -87,6 +87,7 @@ let DateTime DBDPg DBDSQLite + DBIxClassHelpers DigestSHA1 EmailMIME EmailSender diff --git a/src/lib/Hydra/Controller/Jobset.pm b/src/lib/Hydra/Controller/Jobset.pm index 20a52f6f..bc7d7444 100644 --- a/src/lib/Hydra/Controller/Jobset.pm +++ b/src/lib/Hydra/Controller/Jobset.pm @@ -371,6 +371,12 @@ sub errors_GET { $c->stash->{template} = 'eval-error.tt'; + my $jobsetName = $c->stash->{params}->{name}; + $c->stash->{jobset} = $c->stash->{project}->jobsets->find( + { name => $jobsetName }, + { '+columns' => { 'errormsg' => 'errormsg' } } + ); + $self->status_ok($c, entity => $c->stash->{jobset}); } diff --git a/src/lib/Hydra/Controller/JobsetEval.pm b/src/lib/Hydra/Controller/JobsetEval.pm index aca03d72..643a516c 100644 --- a/src/lib/Hydra/Controller/JobsetEval.pm +++ b/src/lib/Hydra/Controller/JobsetEval.pm @@ -93,6 +93,8 @@ sub errors_GET { $c->stash->{template} = 'eval-error.tt'; + $c->stash->{eval} = $c->model('DB::JobsetEvals')->find($c->stash->{eval}->id, { prefetch => 'evaluationerror' }); + $self->status_ok($c, entity => $c->stash->{eval}); } diff --git a/src/lib/Hydra/Helper/Nix.pm b/src/lib/Hydra/Helper/Nix.pm index 71a8a7d7..ef85a535 100644 --- a/src/lib/Hydra/Helper/Nix.pm +++ b/src/lib/Hydra/Helper/Nix.pm @@ -286,8 +286,7 @@ sub getEvals { my @evals = $evals_result_set->search( { hasnewbuilds => 1 }, - { order_by => "$me.id DESC", rows => $rows, offset => $offset - , prefetch => { evaluationerror => [ ] } }); + { order_by => "$me.id DESC", rows => $rows, offset => $offset }); my @res = (); my $cache = {}; diff --git a/src/lib/Hydra/Schema/Result/EvaluationErrors.pm b/src/lib/Hydra/Schema/Result/EvaluationErrors.pm index 7033fa5e..f6cc48db 100644 --- a/src/lib/Hydra/Schema/Result/EvaluationErrors.pm +++ b/src/lib/Hydra/Schema/Result/EvaluationErrors.pm @@ -105,4 +105,6 @@ __PACKAGE__->add_column( "+id" => { retrieve_on_insert => 1 } ); +__PACKAGE__->mk_group_accessors('column' => 'has_error'); + 1; diff --git a/src/lib/Hydra/Schema/Result/Jobsets.pm b/src/lib/Hydra/Schema/Result/Jobsets.pm index cd704ac8..aee87e00 100644 --- a/src/lib/Hydra/Schema/Result/Jobsets.pm +++ b/src/lib/Hydra/Schema/Result/Jobsets.pm @@ -386,6 +386,8 @@ __PACKAGE__->add_column( "+id" => { retrieve_on_insert => 1 } ); +__PACKAGE__->mk_group_accessors('column' => 'has_error'); + sub supportsDynamicRunCommand { my ($self) = @_; diff --git a/src/lib/Hydra/Schema/ResultSet/EvaluationErrors.pm b/src/lib/Hydra/Schema/ResultSet/EvaluationErrors.pm new file mode 100644 index 00000000..a4c6d955 --- /dev/null +++ b/src/lib/Hydra/Schema/ResultSet/EvaluationErrors.pm @@ -0,0 +1,30 @@ +package Hydra::Schema::ResultSet::EvaluationErrors; + +use strict; +use utf8; +use warnings; + +use parent 'DBIx::Class::ResultSet'; + +use Storable qw(dclone); + +__PACKAGE__->load_components('Helper::ResultSet::RemoveColumns'); + +# Exclude expensive error message values unless explicitly requested, and +# replace them with a summary field describing their presence/absence. +sub search_rs { + my ( $class, $query, $attrs ) = @_; + + if ($attrs) { + $attrs = dclone($attrs); + } + + unless (exists $attrs->{'select'} || exists $attrs->{'columns'}) { + $attrs->{'+columns'}->{'has_error'} = "errormsg != ''"; + } + unless (exists $attrs->{'+columns'}->{'errormsg'}) { + push @{ $attrs->{'remove_columns'} }, 'errormsg'; + } + + return $class->next::method($query, $attrs); +} diff --git a/src/lib/Hydra/Schema/ResultSet/Jobsets.pm b/src/lib/Hydra/Schema/ResultSet/Jobsets.pm new file mode 100644 index 00000000..1b2a12e3 --- /dev/null +++ b/src/lib/Hydra/Schema/ResultSet/Jobsets.pm @@ -0,0 +1,30 @@ +package Hydra::Schema::ResultSet::Jobsets; + +use strict; +use utf8; +use warnings; + +use parent 'DBIx::Class::ResultSet'; + +use Storable qw(dclone); + +__PACKAGE__->load_components('Helper::ResultSet::RemoveColumns'); + +# Exclude expensive error message values unless explicitly requested, and +# replace them with a summary field describing their presence/absence. +sub search_rs { + my ( $class, $query, $attrs ) = @_; + + if ($attrs) { + $attrs = dclone($attrs); + } + + unless (exists $attrs->{'select'} || exists $attrs->{'columns'}) { + $attrs->{'+columns'}->{'has_error'} = "errormsg != ''"; + } + unless (exists $attrs->{'+columns'}->{'errormsg'}) { + push @{ $attrs->{'remove_columns'} }, 'errormsg'; + } + + return $class->next::method($query, $attrs); +} diff --git a/src/root/common.tt b/src/root/common.tt index 48655878..12962c2c 100644 --- a/src/root/common.tt +++ b/src/root/common.tt @@ -513,7 +513,7 @@ BLOCK renderEvals %] ELSE %] - [% END %] - [% IF eval.evaluationerror.errormsg %] + [% IF eval.evaluationerror.has_error %] Eval Errors [% END %] @@ -639,7 +639,7 @@ BLOCK renderJobsetOverview %] [% HTML.escape(j.description) %] [% IF j.lastcheckedtime; INCLUDE renderDateTime timestamp = j.lastcheckedtime; - IF j.errormsg || j.fetcherrormsg; %] Error[% END; + IF j.has_error || j.fetcherrormsg; %] Error[% END; ELSE; "-"; END %] [% IF j.get_column('nrtotal') > 0 %] diff --git a/src/root/jobset-eval.tt b/src/root/jobset-eval.tt index 146878f2..f0b92f97 100644 --- a/src/root/jobset-eval.tt +++ b/src/root/jobset-eval.tt @@ -90,7 +90,7 @@ c.uri_for(c.controller('JobsetEval').action_for('view'), [% END %]
  • Inputs
    • - [% IF eval.evaluationerror.errormsg %] + [% IF eval.evaluationerror.has_error %]
  • Evaluation Errors
    • [% END %] @@ -165,7 +165,7 @@ c.uri_for(c.controller('JobsetEval').action_for('view'), [% END %] - [% IF eval.evaluationerror.errormsg %] + [% IF eval.evaluationerror.has_error %]
    diff --git a/src/root/jobset.tt b/src/root/jobset.tt index 562b6beb..0f60b8e0 100644 --- a/src/root/jobset.tt +++ b/src/root/jobset.tt @@ -61,7 +61,7 @@ [% END %]
  • Evaluations
    • - [% IF jobset.errormsg || jobset.fetcherrormsg %] + [% IF jobset.has_error || jobset.fetcherrormsg %]
  • Evaluation Errors
    • [% END %]
  • Jobs
    • @@ -79,7 +79,7 @@ Last checked: [% IF jobset.lastcheckedtime %] - [% INCLUDE renderDateTime timestamp = jobset.lastcheckedtime %], [% IF jobset.errormsg || jobset.fetcherrormsg %]with errors![% ELSE %]no errors[% END %] + [% INCLUDE renderDateTime timestamp = jobset.lastcheckedtime %], [% IF jobset.has_error || jobset.fetcherrormsg %]with errors![% ELSE %]no errors[% END %] [% ELSE %] never [% END %] @@ -117,7 +117,7 @@ - [% IF jobset.errormsg || jobset.fetcherrormsg %] + [% IF jobset.has_error || jobset.fetcherrormsg %]
    diff --git a/t/evaluator/evaluate-constituents-broken.t b/t/evaluator/evaluate-constituents-broken.t index ed25d192..4014f09f 100644 --- a/t/evaluator/evaluate-constituents-broken.t +++ b/t/evaluator/evaluate-constituents-broken.t @@ -22,7 +22,7 @@ like( "The stderr record includes a relevant error message" ); -$jobset->discard_changes; # refresh from DB +$jobset->discard_changes({ '+columns' => {'errormsg' => 'errormsg'} }); # refresh from DB like( $jobset->errormsg, qr/aggregate job ‘mixed_aggregate’ failed with the error: constituentA: does not exist/, diff --git a/t/lib/CliRunners.pm b/t/lib/CliRunners.pm index 885f2ae4..8c53b551 100644 --- a/t/lib/CliRunners.pm +++ b/t/lib/CliRunners.pm @@ -14,7 +14,7 @@ our @EXPORT = qw( sub evalSucceeds { my ($jobset) = @_; my ($res, $stdout, $stderr) = captureStdoutStderr(60, ("hydra-eval-jobset", $jobset->project->name, $jobset->name)); - $jobset->discard_changes; # refresh from DB + $jobset->discard_changes({ '+columns' => {'errormsg' => 'errormsg'} }); # refresh from DB if ($res) { chomp $stdout; chomp $stderr; utf8::decode($stdout) or die "Invalid unicode in stdout."; @@ -29,7 +29,7 @@ sub evalSucceeds { sub evalFails { my ($jobset) = @_; my ($res, $stdout, $stderr) = captureStdoutStderr(60, ("hydra-eval-jobset", $jobset->project->name, $jobset->name)); - $jobset->discard_changes; # refresh from DB + $jobset->discard_changes({ '+columns' => {'errormsg' => 'errormsg'} }); # refresh from DB if (!$res) { chomp $stdout; chomp $stderr; utf8::decode($stdout) or die "Invalid unicode in stdout."; diff --git a/t/queue-runner/direct-indirect-constituents.t b/t/queue-runner/direct-indirect-constituents.t index 35370450..a017c76f 100644 --- a/t/queue-runner/direct-indirect-constituents.t +++ b/t/queue-runner/direct-indirect-constituents.t @@ -13,7 +13,7 @@ my $constituentBuildA = $builds->{"constituentA"}; my $constituentBuildB = $builds->{"constituentB"}; my $eval = $constituentBuildA->jobsetevals->first(); -is($eval->evaluationerror->errormsg, ""); +is($eval->evaluationerror->has_error, 0); subtest "Verifying the direct aggregate" => sub { my $aggBuild = $builds->{"direct_aggregate"};