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

cli: Add "local" network to launcher, and --state-dir for the database #377

Merged
merged 4 commits into from
Jun 6, 2019

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Jun 6, 2019

Relates to issue #154
This branch is based on the branch of #377

Overview

  • The launcher needs to pass a database filename to the wallet server.
  • It does this using --state-dir. That can also control where cardano-http-bridge puts its files.
  • Also, our cardano-http-bridge fork supports --template local, but cardano-wallet-launcher wouldn't recognise that as a valid network.

@@ -74,15 +81,17 @@ main = do
when (args `isPresent` (shortOption 'h')) $ help cli
when (args `isPresent` (longOption "version")) $ putStrLn getVersion

network <- args `parseArg` longOption "network"
let stateDir = args `getArg` (longOption "state-dir")
let network = fromMaybe "testnet" $ args `getArg` (longOption "network")
Copy link
Member

Choose a reason for hiding this comment

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

Just a small point, but parentheses are not actually needed in the above two lines.

@@ -62,9 +68,10 @@ Usage:
cardano-wallet-launcher --version

Options:
--network <STRING> testnet, staging, or mainnet [default: testnet]
--network <STRING> testnet, staging, mainnet, or local [default: testnet]
Copy link
Contributor

Choose a reason for hiding this comment

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

why is local coming back?

Copy link
Member

Choose a reason for hiding this comment

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

Because of Daedalus integrating with the wallet backend. They also need to be able to spin-up a local cluster for their own testing.

@@ -74,15 +81,17 @@ main = do
when (args `isPresent` (shortOption 'h')) $ help cli
when (args `isPresent` (longOption "version")) $ putStrLn getVersion

network <- args `parseArg` longOption "network"
let stateDir = args `getArg` (longOption "state-dir")
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to add integration test that'd check if the directory and contents are created correctly.
Could be here -> https://github.com/input-output-hk/cardano-wallet/tree/master/lib/http-bridge/test/integration/Test/Integration/Scenario/CLI

@KtorZ KtorZ changed the base branch from rvl/sqlite-db-layer to master June 6, 2019 11:28
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

LGTM. Rebased on top of latest master. Note that this PR also includes the change from #374.

@KtorZ KtorZ force-pushed the rvl/launcher-fix branch 2 times, most recently from a0a7a61 to 264a459 Compare June 6, 2019 11:51
@KtorZ KtorZ merged commit 3106a4a into master Jun 6, 2019
@KtorZ KtorZ deleted the rvl/launcher-fix branch June 6, 2019 13:51
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.

None yet

4 participants