Skip to content

Commit 4f09fb5

Browse files
delrothmweinelt
authored andcommitted
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.
1 parent ecd7572 commit 4f09fb5

14 files changed

+85
-13
lines changed

package.nix

+1
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ let
8989
DateTime
9090
DBDPg
9191
DBDSQLite
92+
DBIxClassHelpers
9293
DigestSHA1
9394
EmailMIME
9495
EmailSender

src/lib/Hydra/Controller/Jobset.pm

+6
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,12 @@ sub errors_GET {
371371

372372
$c->stash->{template} = 'eval-error.tt';
373373

374+
my $jobsetName = $c->stash->{params}->{name};
375+
$c->stash->{jobset} = $c->stash->{project}->jobsets->find(
376+
{ name => $jobsetName },
377+
{ '+columns' => { 'errormsg' => 'errormsg' } }
378+
);
379+
374380
$self->status_ok($c, entity => $c->stash->{jobset});
375381
}
376382

src/lib/Hydra/Controller/JobsetEval.pm

+2
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ sub errors_GET {
9393

9494
$c->stash->{template} = 'eval-error.tt';
9595

96+
$c->stash->{eval} = $c->model('DB::JobsetEvals')->find($c->stash->{eval}->id, { prefetch => 'evaluationerror' });
97+
9698
$self->status_ok($c, entity => $c->stash->{eval});
9799
}
98100

src/lib/Hydra/Helper/Nix.pm

+1-2
Original file line numberDiff line numberDiff line change
@@ -296,8 +296,7 @@ sub getEvals {
296296

297297
my @evals = $evals_result_set->search(
298298
{ hasnewbuilds => 1 },
299-
{ order_by => "$me.id DESC", rows => $rows, offset => $offset
300-
, prefetch => { evaluationerror => [ ] } });
299+
{ order_by => "$me.id DESC", rows => $rows, offset => $offset });
301300
my @res = ();
302301
my $cache = {};
303302

src/lib/Hydra/Schema/Result/EvaluationErrors.pm

+2
Original file line numberDiff line numberDiff line change
@@ -105,4 +105,6 @@ __PACKAGE__->add_column(
105105
"+id" => { retrieve_on_insert => 1 }
106106
);
107107

108+
__PACKAGE__->mk_group_accessors('column' => 'has_error');
109+
108110
1;

src/lib/Hydra/Schema/Result/Jobsets.pm

+2
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,8 @@ __PACKAGE__->add_column(
386386
"+id" => { retrieve_on_insert => 1 }
387387
);
388388

389+
__PACKAGE__->mk_group_accessors('column' => 'has_error');
390+
389391
sub supportsDynamicRunCommand {
390392
my ($self) = @_;
391393

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package Hydra::Schema::ResultSet::EvaluationErrors;
2+
3+
use strict;
4+
use utf8;
5+
use warnings;
6+
7+
use parent 'DBIx::Class::ResultSet';
8+
9+
use Storable qw(dclone);
10+
11+
__PACKAGE__->load_components('Helper::ResultSet::RemoveColumns');
12+
13+
# Exclude expensive error message values unless explicitly requested, and
14+
# replace them with a summary field describing their presence/absence.
15+
sub search_rs {
16+
my ( $class, $query, $attrs ) = @_;
17+
18+
if ($attrs) {
19+
$attrs = dclone($attrs);
20+
}
21+
22+
unless (exists $attrs->{'select'} || exists $attrs->{'columns'}) {
23+
$attrs->{'+columns'}->{'has_error'} = "errormsg != ''";
24+
}
25+
unless (exists $attrs->{'+columns'}->{'errormsg'}) {
26+
push @{ $attrs->{'remove_columns'} }, 'errormsg';
27+
}
28+
29+
return $class->next::method($query, $attrs);
30+
}
+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package Hydra::Schema::ResultSet::Jobsets;
2+
3+
use strict;
4+
use utf8;
5+
use warnings;
6+
7+
use parent 'DBIx::Class::ResultSet';
8+
9+
use Storable qw(dclone);
10+
11+
__PACKAGE__->load_components('Helper::ResultSet::RemoveColumns');
12+
13+
# Exclude expensive error message values unless explicitly requested, and
14+
# replace them with a summary field describing their presence/absence.
15+
sub search_rs {
16+
my ( $class, $query, $attrs ) = @_;
17+
18+
if ($attrs) {
19+
$attrs = dclone($attrs);
20+
}
21+
22+
unless (exists $attrs->{'select'} || exists $attrs->{'columns'}) {
23+
$attrs->{'+columns'}->{'has_error'} = "errormsg != ''";
24+
}
25+
unless (exists $attrs->{'+columns'}->{'errormsg'}) {
26+
push @{ $attrs->{'remove_columns'} }, 'errormsg';
27+
}
28+
29+
return $class->next::method($query, $attrs);
30+
}

src/root/common.tt

+2-2
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ BLOCK renderEvals %]
513513
ELSE %]
514514
-
515515
[% END %]
516-
[% IF eval.evaluationerror.errormsg %]
516+
[% IF eval.evaluationerror.has_error %]
517517
<span class="badge badge-warning">Eval Errors</span>
518518
[% END %]
519519
</td>
@@ -639,7 +639,7 @@ BLOCK renderJobsetOverview %]
639639
<td>[% HTML.escape(j.description) %]</td>
640640
<td>[% IF j.lastcheckedtime;
641641
INCLUDE renderDateTime timestamp = j.lastcheckedtime;
642-
IF j.errormsg || j.fetcherrormsg; %]&nbsp;<span class = 'badge badge-warning'>Error</span>[% END;
642+
IF j.has_error || j.fetcherrormsg; %]&nbsp;<span class = 'badge badge-warning'>Error</span>[% END;
643643
ELSE; "-";
644644
END %]</td>
645645
[% IF j.get_column('nrtotal') > 0 %]

src/root/jobset-eval.tt

+2-2
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ c.uri_for(c.controller('JobsetEval').action_for('view'),
9090
[% END %]
9191
<li class="nav-item"><a class="nav-link" href="#tabs-inputs" data-toggle="tab">Inputs</a></li>
9292

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

168-
[% IF eval.evaluationerror.errormsg %]
168+
[% IF eval.evaluationerror.has_error %]
169169
<div id="tabs-errors" class="tab-pane">
170170
<iframe src="[% c.uri_for(c.controller('JobsetEval').action_for('errors'), [eval.id], params) %]" loading="lazy" frameBorder="0" width="100%"></iframe>
171171
</div>

src/root/jobset.tt

+3-3
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
[% END %]
6262

6363
<li class="nav-item"><a class="nav-link active" href="#tabs-evaluations" data-toggle="tab">Evaluations</a></li>
64-
[% IF jobset.errormsg || jobset.fetcherrormsg %]
64+
[% IF jobset.has_error || jobset.fetcherrormsg %]
6565
<li class="nav-item"><a class="nav-link" href="#tabs-errors" data-toggle="tab"><span class="text-warning">Evaluation Errors</span></a></li>
6666
[% END %]
6767
<li class="nav-item"><a class="nav-link" href="#tabs-jobs" data-toggle="tab">Jobs</a></li>
@@ -79,7 +79,7 @@
7979
<th>Last checked:</th>
8080
<td>
8181
[% IF jobset.lastcheckedtime %]
82-
[% INCLUDE renderDateTime timestamp = jobset.lastcheckedtime %], [% IF jobset.errormsg || jobset.fetcherrormsg %]<em class="text-warning">with errors!</em>[% ELSE %]<em>no errors</em>[% END %]
82+
[% INCLUDE renderDateTime timestamp = jobset.lastcheckedtime %], [% IF jobset.has_error || jobset.fetcherrormsg %]<em class="text-warning">with errors!</em>[% ELSE %]<em>no errors</em>[% END %]
8383
[% ELSE %]
8484
<em>never</em>
8585
[% END %]
@@ -117,7 +117,7 @@
117117

118118
</div>
119119

120-
[% IF jobset.errormsg || jobset.fetcherrormsg %]
120+
[% IF jobset.has_error || jobset.fetcherrormsg %]
121121
<div id="tabs-errors" class="tab-pane">
122122
<iframe src="[% c.uri_for('/jobset' project.name jobset.name "errors") %]" loading="lazy" frameBorder="0" width="100%"></iframe>
123123
</div>

t/evaluator/evaluate-constituents-broken.t

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ like(
2222
"The stderr record includes a relevant error message"
2323
);
2424

25-
$jobset->discard_changes; # refresh from DB
25+
$jobset->discard_changes({ '+columns' => {'errormsg' => 'errormsg'} }); # refresh from DB
2626
like(
2727
$jobset->errormsg,
2828
qr/aggregate job ‘mixed_aggregate’ failed with the error: constituentA: does not exist/,

t/lib/CliRunners.pm

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ our @EXPORT = qw(
1414
sub evalSucceeds {
1515
my ($jobset) = @_;
1616
my ($res, $stdout, $stderr) = captureStdoutStderr(60, ("hydra-eval-jobset", $jobset->project->name, $jobset->name));
17-
$jobset->discard_changes; # refresh from DB
17+
$jobset->discard_changes({ '+columns' => {'errormsg' => 'errormsg'} }); # refresh from DB
1818
if ($res) {
1919
chomp $stdout; chomp $stderr;
2020
utf8::decode($stdout) or die "Invalid unicode in stdout.";
@@ -29,7 +29,7 @@ sub evalSucceeds {
2929
sub evalFails {
3030
my ($jobset) = @_;
3131
my ($res, $stdout, $stderr) = captureStdoutStderr(60, ("hydra-eval-jobset", $jobset->project->name, $jobset->name));
32-
$jobset->discard_changes; # refresh from DB
32+
$jobset->discard_changes({ '+columns' => {'errormsg' => 'errormsg'} }); # refresh from DB
3333
if (!$res) {
3434
chomp $stdout; chomp $stderr;
3535
utf8::decode($stdout) or die "Invalid unicode in stdout.";

t/queue-runner/direct-indirect-constituents.t

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ my $constituentBuildA = $builds->{"constituentA"};
1313
my $constituentBuildB = $builds->{"constituentB"};
1414

1515
my $eval = $constituentBuildA->jobsetevals->first();
16-
is($eval->evaluationerror->errormsg, "");
16+
is($eval->evaluationerror->has_error, 0);
1717

1818
subtest "Verifying the direct aggregate" => sub {
1919
my $aggBuild = $builds->{"direct_aggregate"};

0 commit comments

Comments
 (0)