Add nix-store-gcs-proxy service #79925
Conversation
|
cc @andir |
There was a problem hiding this comment.
How and where does this get credentials from? I think we should add a few more lines of documentation either in the options and/or the nixos documentation.
As far as I know it tries to retrieve those credentials from some environment variable and some well known file system location?
Also commented on the test that should probably be improved to do something.
mrkkrp
force-pushed
the
mk/add-nix-store-gcs-proxy-service
branch
from
56e0b18
to
450d3da
Compare
|
@andir What do you mean by "credentials"? Accessibility of storage bucket is controlled (at least in our case) by service account. If you mean nix signing key, then it is provided like this in the post build hook: Added the test to |
Where does that service account come from if I run this on my laptop? |
|
@andir Apparently you can't really run it without real credentials. If you have credentials you can provide them in one of the ways described here: https://cloud.google.com/docs/authentication/production This is why I had to abandon writing "real" tests. |
mrkkrp
force-pushed
the
mk/add-nix-store-gcs-proxy-service
branch
from
450d3da
to
7f54f38
Compare
mrkkrp
force-pushed
the
mk/add-nix-store-gcs-proxy-service
branch
from
7f54f38
to
4520bed
Compare
nixos/tests/nix-store-gcs-proxy.nix
Outdated
|
|
||
| testScript = '' | ||
| start_all() | ||
| # the nix-store-gcs-proxy executable needs valid credentials to be |
There was a problem hiding this comment.
Then this test is basically useless? It doesn't even wait for the VM to start. It just starts it and then terminates.
cc @zimbatm Since you authored (most of?) the proxy: Is there something sensible we can actually test here? I would really like the daemon come up and respond to http requests.
There was a problem hiding this comment.
I guess we could add a /_health end-point like this: tweag/nix-store-gcs-proxy#7 ? Ideally the _health endpoint also checks that it has access to the bucket but in this context it would be hard to emulate that.
There was a problem hiding this comment.
Let's just drop this test as it's not really useful in its current form.
There was a problem hiding this comment.
It could be emulated using something like that https://github.com/fsouza/fake-gcs-server.
For the time being this might be too complicated...
There was a problem hiding this comment.
Yeah, I think we have been setting the bar of acceptance too high and it's unfair to @mrkkrp
|
Commit message should read "module/nix-store-gcs-proxy: init" per Contributing. |
mrkkrp
force-pushed
the
mk/add-nix-store-gcs-proxy-service
branch
2 times, most recently
from
d2e16d1
to
cb8d40d
Compare
|
On 03:00 21.02.20, Mark Karpov wrote:
mrkkrp commented on this pull request.
> @@ -0,0 +1,74 @@
+{ config, lib, pkgs, ... }:
+
+with lib;
+
+let
+ opts = { name, config, ... }: {
+ options = {
+ enable = mkOption {
AFAIU `mkEnableOption` sets `false` as the default, whereas here we want it to be `true`.
That is indeed true. Sorry for the noise then :)
|
|
Is there anything else I should adjust to get this merged? |
|
On 04:57 24.02.20, Mark Karpov wrote:
Is there anything else I should adjust to get this merged?
Yes, the test still needs some action. It currently starts the qemu
process and then just terminates. There is nothing it tests besides eval
of the module and for that it rather expensive.
I commented on the test and @zimbatm responded.
See: https://github.com/NixOS/nixpkgs/pull/79925/files#r382643815
|
|
Agreed, let's merge this without the test. Not all modules need to be thoroughly tested and if it works that's good enough for me. |
|
Let's click "merge" then? |
|
Please remove the test as discussed above. |
mrkkrp
force-pushed
the
mk/add-nix-store-gcs-proxy-service
branch
2 times, most recently
from
2a41ed6
to
6a52c55
Compare
|
@zimbatm Done. |
mrkkrp
force-pushed
the
mk/add-nix-store-gcs-proxy-service
branch
from
6a52c55
to
be8fe89
Compare
mrkkrp
force-pushed
the
mk/add-nix-store-gcs-proxy-service
branch
from
be8fe89
to
96b472e
Compare
Motivation for this change
This adds a NixOS module which provides a service for running
nix-store-gcs-proxyautomatically.Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)