add Kupo package, module, test #60
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brainrake
force-pushed
the
marton/kupo
branch
7 times, most recently
from
ee61a1a to
b69857c
Compare
zmrocze
reviewed
May 8, 2024
modules/kupo.nix
Outdated
| services.kupo = { | ||
| enable = true; | ||
| nodeSocketPath = | ||
| lib.mkIf (!(config.cardano.ogmios.enable or true) || config.cardano.node.enable or false) |
Contributor
There was a problem hiding this comment.
What's the reason for that condition?
Contributor
Author
There was a problem hiding this comment.
- if ogmios is enabled on the same host, kupo should by default connect to ogmios and not use node socket path
- if ogmios is not enabled but cardano-node is enabled, node socket should be used
- if neither is enabled, no default should be set and the user is expected to set either node socket path or ogmios address
Contributor
There was a problem hiding this comment.
But right now if neither is enabled then the default option is set.
so shouldn't it be instead:
! ogmios && cardano.nodeor just
cardano.nodebecause it doesn't harm to set the option if its not used in the case of ogmios enabled (and actually, I've hit this in db-sync - will it not just error with type error when ogmios is enabled but cardano.node is not? then the option is not set and the else clause of mkIf is not of the fitting type path)
Contributor
Author
There was a problem hiding this comment.
- ok, made the condition simpler by just checking
cardano.node.enable - the option type is actually
nullOr pathbecause it is optional, sonullwas fine - i added
or nullbecause of some problem with the documentation generation, looks like it works though - the default value is a path anyway, makes sense to set it as the most common value used
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.