Skip to content

Commit

Permalink
web: replace 'errormsg' with 'errormsg IS NULL' in most cases
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
delroth committed Apr 12, 2024
1 parent 258e931 commit 6189ba9
Show file tree
Hide file tree
Showing 14 changed files with 85 additions and 13 deletions.
1 change: 1 addition & 0 deletions package.nix
Expand Up @@ -87,6 +87,7 @@ let
DateTime
DBDPg
DBDSQLite
DBIxClassHelpers
DigestSHA1
EmailMIME
EmailSender
Expand Down
6 changes: 6 additions & 0 deletions src/lib/Hydra/Controller/Jobset.pm
Expand Up @@ -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});
}

Expand Down
2 changes: 2 additions & 0 deletions src/lib/Hydra/Controller/JobsetEval.pm
Expand Up @@ -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});
}

Expand Down
3 changes: 1 addition & 2 deletions src/lib/Hydra/Helper/Nix.pm
Expand Up @@ -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 = {};

Expand Down
2 changes: 2 additions & 0 deletions src/lib/Hydra/Schema/Result/EvaluationErrors.pm
Expand Up @@ -105,4 +105,6 @@ __PACKAGE__->add_column(
"+id" => { retrieve_on_insert => 1 }
);

__PACKAGE__->mk_group_accessors('column' => 'has_error');

1;
2 changes: 2 additions & 0 deletions src/lib/Hydra/Schema/Result/Jobsets.pm
Expand Up @@ -386,6 +386,8 @@ __PACKAGE__->add_column(
"+id" => { retrieve_on_insert => 1 }
);

__PACKAGE__->mk_group_accessors('column' => 'has_error');

sub supportsDynamicRunCommand {
my ($self) = @_;

Expand Down
30 changes: 30 additions & 0 deletions 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);
}
30 changes: 30 additions & 0 deletions 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);
}
4 changes: 2 additions & 2 deletions src/root/common.tt
Expand Up @@ -513,7 +513,7 @@ BLOCK renderEvals %]
ELSE %]
-
[% END %]
[% IF eval.evaluationerror.errormsg %]
[% IF eval.evaluationerror.has_error %]
<span class="badge badge-warning">Eval Errors</span>
[% END %]
</td>
Expand Down Expand Up @@ -639,7 +639,7 @@ BLOCK renderJobsetOverview %]
<td>[% HTML.escape(j.description) %]</td>
<td>[% IF j.lastcheckedtime;
INCLUDE renderDateTime timestamp = j.lastcheckedtime;
IF j.errormsg || j.fetcherrormsg; %]&nbsp;<span class = 'badge badge-warning'>Error</span>[% END;
IF j.has_error || j.fetcherrormsg; %]&nbsp;<span class = 'badge badge-warning'>Error</span>[% END;
ELSE; "-";
END %]</td>
[% IF j.get_column('nrtotal') > 0 %]
Expand Down
4 changes: 2 additions & 2 deletions src/root/jobset-eval.tt
Expand Up @@ -90,7 +90,7 @@ c.uri_for(c.controller('JobsetEval').action_for('view'),
[% END %]
<li class="nav-item"><a class="nav-link" href="#tabs-inputs" data-toggle="tab">Inputs</a></li>

[% IF eval.evaluationerror.errormsg %]
[% IF eval.evaluationerror.has_error %]
<li class="nav-item"><a class="nav-link" href="#tabs-errors" data-toggle="tab"><span class="text-warning">Evaluation Errors</span></a></li>
[% END %]
</ul>
Expand Down Expand Up @@ -165,7 +165,7 @@ c.uri_for(c.controller('JobsetEval').action_for('view'),
[% END %]
</div>

[% IF eval.evaluationerror.errormsg %]
[% IF eval.evaluationerror.has_error %]
<div id="tabs-errors" class="tab-pane">
<iframe src="[% c.uri_for(c.controller('JobsetEval').action_for('errors'), [eval.id], params) %]" loading="lazy" frameBorder="0" width="100%"></iframe>
</div>
Expand Down
6 changes: 3 additions & 3 deletions src/root/jobset.tt
Expand Up @@ -61,7 +61,7 @@
[% END %]

<li class="nav-item"><a class="nav-link active" href="#tabs-evaluations" data-toggle="tab">Evaluations</a></li>
[% IF jobset.errormsg || jobset.fetcherrormsg %]
[% IF jobset.has_error || jobset.fetcherrormsg %]
<li class="nav-item"><a class="nav-link" href="#tabs-errors" data-toggle="tab"><span class="text-warning">Evaluation Errors</span></a></li>
[% END %]
<li class="nav-item"><a class="nav-link" href="#tabs-jobs" data-toggle="tab">Jobs</a></li>
Expand All @@ -79,7 +79,7 @@
<th>Last checked:</th>
<td>
[% IF jobset.lastcheckedtime %]
[% INCLUDE renderDateTime timestamp = jobset.lastcheckedtime %], [% IF jobset.errormsg || jobset.fetcherrormsg %]<em class="text-warning">with errors!</em>[% ELSE %]<em>no errors</em>[% END %]
[% INCLUDE renderDateTime timestamp = jobset.lastcheckedtime %], [% IF jobset.has_error || jobset.fetcherrormsg %]<em class="text-warning">with errors!</em>[% ELSE %]<em>no errors</em>[% END %]
[% ELSE %]
<em>never</em>
[% END %]
Expand Down Expand Up @@ -117,7 +117,7 @@

</div>

[% IF jobset.errormsg || jobset.fetcherrormsg %]
[% IF jobset.has_error || jobset.fetcherrormsg %]
<div id="tabs-errors" class="tab-pane">
<iframe src="[% c.uri_for('/jobset' project.name jobset.name "errors") %]" loading="lazy" frameBorder="0" width="100%"></iframe>
</div>
Expand Down
2 changes: 1 addition & 1 deletion t/evaluator/evaluate-constituents-broken.t
Expand Up @@ -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/,
Expand Down
4 changes: 2 additions & 2 deletions t/lib/CliRunners.pm
Expand Up @@ -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.";
Expand All @@ -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.";
Expand Down
2 changes: 1 addition & 1 deletion t/queue-runner/direct-indirect-constituents.t
Expand Up @@ -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"};
Expand Down

0 comments on commit 6189ba9

Please sign in to comment.