New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Show evaluation errors #569
Conversation
@@ -341,10 +341,20 @@ sub evals :Local Args(0) { | |||
|
|||
my $evals = $c->model('DB::JobsetEvals'); | |||
|
|||
my @failEvals = [$c->model('DB::Jobsets') | |||
->search({ "me.enabled" => 1, "me.hidden" => 0, |
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.
There are some indentation issues here.
Also, it may be better to do only one search
call, as it is implemented in the search
method below (while I'm really not sure about this:/).
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 not sure quite what you mean about indentation "issues". There are multiple dereferences, and each reference is somewhat long, so this separates each dereference to a separate line. The code works just fine (I run it locally). Was your concern about functionality or just appearance, and did you have a specific recommendation regarding appearance?
In my experience it is typical for ORMs to perform local optimization of the generated SQL statement, and multiple refinements like this are often recommended as a way to specialize a base query. Additionally the SQL server performs optimization on the query, so I don't expect this will result in any performance problems, especially when used with postgres.
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.
Ok, forget my comment about indentation and thx for your answer!
I feel a bit scared about adding this to the load. These pages already do pretty heavy pageloads on hydra.nixos.org -- adding more, heavy queries seem spooky. |
@grahamc Actually, one commit (e7a7bd5) of this PR should not increase the load and is quite nice (it shows evaluation errors from the project view). Regarding the second commit, if we really want this feature, we could add a configuration variable to enable this feature. It would be disable by default and people wanting this feature could explicitly enable it. |
Thanks, I've cherry-picked commit e7a7bd5. |
Considering the concerns on the one hand, and the fact that parts were cherry picked i’ll close this PR. Thank you for the contribution. |
Display evaluation errors on the project summary page and also on the Status:Latest Evaluations page.
In the original form, processing of a jobset can be affected by evaluation errors, but there is no indication of an evaluation error unless you visit the specific jobset page. This modification makes these evaluation errors visible on top-level summaries (in the same places that build failures are shown) so that there is visible indication of a problem that should be fixed.
If there are no errors, there should be no difference in the display from the current output. This only shows additional information when there are evaluation errors.