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.

Sign up for GitHub

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

Improve --help ux for Canton sandbox #12990

Merged
merged 2 commits into from
Feb 21, 2022
Merged

Improve --help ux for Canton sandbox #12990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e282458
Improve --help ux for Canton sandbox
cocreature Feb 17, 2022
81ec8ac
.
cocreature Feb 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions daml-assistant/daml-helper/src/DA/Daml/Helper/Main.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,23 @@ commandParser = subparser $ fold
, help "Timeout of gRPC operations in seconds. Defaults to 60s. Must be > 0."
]

cantonHelpSwitch =
switch $
long "canton-help" <>
help "Display the help of the underlying Canton JAR instead of the Sandbox wrapper. This is only required for advanced options."

-- These options are common enough that we want them to show up in --help instead of only in
-- --canton-help.
cantonConfigOpts =
many $
option str $
long "config" <>
short 'c' <>
metavar "FILE" <>
help (unwords [ "Set configuration file(s)."
, "If several configuration files assign values to the same key, the last value is taken."
])

cantonSandboxCmd = do
cantonOptions <- do
cantonLedgerApi <- option auto (long "port" <> value 6865)
Expand All @@ -432,6 +449,8 @@ commandParser = subparser $ fold
(flag' True (long "static-time") <|>
flag' False (long "wall-clock-time") <|>
pure False)
cantonHelp <- cantonHelpSwitch
cantonConfigFiles <- cantonConfigOpts
pure CantonOptions{..}
portFileM <- optional $ option str (long "port-file" <> metavar "PATH"
<> help "File to write ledger API port when ready")
Expand Down
2 changes: 2 additions & 0 deletions daml-assistant/daml-helper/src/DA/Daml/Helper/Start.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ determineCantonOptions ledgerApiSpec SandboxCantonPortSpec{..} portFile = do
cantonDomainAdminApi <- getPortForSandbox FreePort domainAdminApiSpec
let cantonPortFileM = Just portFile -- TODO allow canton port file to be passed in from command line?
let cantonStaticTime = StaticTime False
let cantonHelp = False
let cantonConfigFiles = []
pure CantonOptions {..}

withSandbox :: StartOptions -> FilePath -> [String] -> (Process () () () -> SandboxPort -> IO a) -> IO a
Expand Down
6 changes: 5 additions & 1 deletion daml-assistant/daml-helper/src/DA/Daml/Helper/Util.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,9 @@ withCantonSandbox options remainingArgs k = do
let cantonJar = sdkPath </> "canton" </> "canton.jar"
withTempFile $ \config -> do
BSL.writeFile config (cantonConfig options)
withJar cantonJar [] ("daemon" : "-c" : config : "--auto-connect-local" : remainingArgs) k
let args | cantonHelp options = ["--help"]
| otherwise = concatMap (\f -> ["-c", f]) (cantonConfigFiles options)
withJar cantonJar [] ("daemon" : "-c" : config : "--auto-connect-local" : (args <> remainingArgs)) k

-- | Obtain a path to use as canton portfile, and give updated options.
withCantonPortFile :: CantonOptions -> (CantonOptions -> FilePath -> IO a) -> IO a
Expand All @@ -281,6 +283,8 @@ data CantonOptions = CantonOptions
, cantonDomainAdminApi :: Int
, cantonPortFileM :: Maybe FilePath
, cantonStaticTime :: StaticTime
, cantonHelp :: Bool
, cantonConfigFiles :: [FilePath]
}

cantonConfig :: CantonOptions -> BSL.ByteString
Expand Down
4 changes: 2 additions & 2 deletions docs/source/tools/sandbox.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,8 @@ To start Sandbox, run: ``daml sandbox [options] [-c canton.config]``.

To see all the available options, run ``daml sandbox --help``. Note
that this will show you the options of the Sandbox wrapper around
Canton. To see options of the underlying Canton runner, use ``daml
sandbox -- -- --help``.
Canton. To see options of the underlying Canton runner, use
``daml sandbox --canton-help``.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we rather consider going the git way and doing an help subcommand? I.e. daml help sandbox

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don’t see how that helps. You still have help for the sandbox wrapper and for the underlying canton JAR. Git doesn’t have that issue git status --help and git help status are the same help text.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don’t see how that helps. You still have help for the sandbox wrapper and for the underlying canton JAR. Git doesn’t have that issue git status --help and git help status are the same help text.

Yes, but it doesn't have git status --status-help. I would agree with you if you could have daml sandbox --help. Right now as a user I would prefer to see the help text for Canton by typing that. I expect some confusion from users over using daml sandbox --help vs. daml sandbox --canton-help.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A few comments to explain why we’re in this situation: Canton itself does not have any special sandbox mode or even a separate JAR. So we need some wrapper that turns Canton into a somewhat usable sandbox by specifying certain options and config files. This wrapper has to make some choices like ports. It also provides some UX niceties that people expect in sandbox and make it easier to migrate like options to turn on static time. The idea here is that in 95% of cases, you only ever need to interact with the wrapper directly and you don’t have to care that there is an underlying Canton JAR.

However, Canton has thousands of options and we clearly don’t want to replicate all of them in the wrapper. So for advanced usages you might need to look at the Canton help.

I fully agree that the UX isn’t great and somewhat leaks how things are implemented internally. I don’t really have a better suggestion though. We can try to parse Canton’s --help output and integrate it but that seems super hacky and is bound to fail in the future. We could make daml sandbox -- --help work but I’m not sure that’s really better (it still leaks implementation details of there being an underlying wrapper) and it’s not discoverable. --canton-help has the advantage that it shows up if you just type --help.


Metrics
*******
Expand Down