Skip to content
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

ca-derivations: fix rendering issue #1374

Merged

Conversation

Mindavi
Copy link
Contributor

@Mindavi Mindavi commented Apr 3, 2024

See commit messages for some more background and analysis.

I set up my hydra server with the recent content-addressed derivations support, and ran into some /build/<id> pages that didn't want to render correctly (not only ca-derivations, other pages also suffered from this). I think I found the issue and have worked around/resolved it.

Also adds a test that verifies that the page doesn't load properly before the patch is applied. In the next commit this bug is resolved.

[  FAIL  ]  job  1  + The 'build' page for build 'caDependingOnFailingCA' should load properly

Mindavi added 3 commits April 3, 2024 22:45
This uncovers an issue with the front-end.
When content addressed derivations are built on the hydra server,
one may run into an issue where some builds suddenly don't load anymore.

This seems to be caused by outPaths that are NULL (which is
allowed for ca-derivations). Filter them out to prevent querying the
database for them, which is not supported by the database abstraction
layer that's currently in use.

On my instance this appears to resolve the issue.
I feel like I might be doing this at the wrong abstraction layer, but on
the other hand -- it seems to resolve it and it also doesn't really look
like it will hurt anything.

The test added in a previous commit uncovers this issue, and this commit
resolves it. So I'm happy with this patch for now.

The issue I was seeing on my server:

hydra-server[2549]: [error] Couldn't render template "undef error - DBIx::Class::SQLMaker::ClassicExtensions::puke(): Fatal: NULL-within-IN not implemented: The upcoming SQL::Abstract::Classic 2.0 will emit the logically correct SQL instead of raising this exception. at /nix/store/<hash>-hydra-unstable-2024-03-08_nix_2_20/libexec/hydra/lib/Hydra/Helper/Nix.pm line 190

See also short discussion here: NixOS/nixpkgs#297392 (comment)
Mindavi added a commit to Mindavi/nixos-configuration that referenced this pull request Apr 3, 2024
Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice PR!

I think is fine to put that check there.

@Ericson2314 Ericson2314 merged commit 8b48579 into NixOS:master Apr 18, 2024
1 check passed
@Mindavi Mindavi deleted the bugfix/rendering-issue-content-addressed branch April 18, 2024 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants