-
-
Notifications
You must be signed in to change notification settings - Fork 309
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
lazy-load evaluation errors #1363
base: master
Are you sure you want to change the base?
Conversation
[% IF eval.evaluationerror.errormsg %] | ||
<div id="tabs-errors" class="tab-pane"> | ||
<p>Errors occurred at [% INCLUDE renderDateTime timestamp=(eval.evaluationerror.errortime || eval.timestamp) %].</p> | ||
<div class="card bg-light"><div class="card-body"><pre>[% HTML.escape(eval.evaluationerror.errormsg) %]</pre></div></div> | ||
</div> | ||
[% END %] | ||
|
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.
Evals did in fact ship the eval error log twice, so we're killing that as well. 🤷
Future improvement: It's likely a slightly more involved fix though, just mentioning it in case you're interested :p |
To whoever will merge this: please cherry-pick it on the 2.19 branch so we can easily apply it to hydra.nixos.org. |
FWIW we've tested this on hydra.nixos.org for a few days and everything worked fine. @Ericson2314 can you merge this please? |
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.
fwiw evaluationerrors
is also part of prefetch
in Nix::getEvals
, if we want to get rid of it in the first query, we'll probably need to touch that part as well.
That said, I only know because I failed to read the entire conversation first and wondered how lazy loading works on ORM-side (which made me enable query logging in my postgresql 😅).
Anyways, the change looks good to me and works fine on my personal instance 👍
@Ma27 If you know what to do, please give it a shot. |
The JSON API for jobsets/evals also ships the whole eval errors string, which is utterly pointless. Can we rip that out as well?
|
I'll happily do that, but it may take me a few days. |
Closes #1362