-
Notifications
You must be signed in to change notification settings - Fork 751
cardano-testnet: allow to take node configuration file as input #6148
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
Conversation
Jimbo4350
left a comment
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.
LGTM 👍
| else | ||
| -- Here we forbid mixing defaulting of genesis files with providing a user node configuration file: | ||
| -- | ||
| -- Because of this check, either the user passes a node configuration file AND he passes all the genesis files. |
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.
👍
|
|
||
| -- Make Non P2P topology files | ||
| forM_ (zip [1..] portNumbers) $ \(i, myPortNumber) -> do | ||
| -- TODO: if the user provided its own configuration file, and requested a P2P topology file, |
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.
Can you turn this into an issue so we can track it?
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.
Done here: #6150
| <> testnetNodeExtraCliArgs nodeOptions | ||
| <> maybe [] extraCliArgs nodeOptions | ||
| pure $ eRuntime <&> \rt -> rt{poolKeys=mKeys} | ||
| -- TODO log the node's pid in a file. This is useful for killing the node later. |
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.
If this is deserving of an issue can you create it?
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.
Follow-up issue created: #6149
2f5e95e to
3ae3a62
Compare
Warning
If this design is chosen (over #6103), then cardano-foundation/developer-portal#1426 needs to be amended correspondingly
Description
Fixes #6069
Fixes #6137
Variant of #6103 where passing the node configuration file and the genesis files are tied:
cardano-testnetgenerates everything for youFollow-up issues to create
@Jimbo4350> please tick them if you agree 🙂 I'll create them
pTestnetNodeOptionsreturn a list ofTestnetNodeOptionandTestnetNodeOptionbeUserProvidedNodeOptions Filepath | SpoNodeOptions [Srting] | RelayNodeOptions [ String]. In other words merge the typesTestnetNodeOptionsandAutomaticNodeOption. This will generalize the behavior so that the user can create multiple nodes, no matter whether they are using custom options or using default behavior (SpoorRelay). This will require a bit of generalization in Cardano.hs but not much.ChangeDecided againstcardanoTestnetto takeFilepathfor genesis files instead of Haskell values, maybe?runTestnet: don't tie it with H.Integration #6122Checklist