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

soroban network start followup improvements #1230

Closed
5 of 7 tasks
elizabethengelman opened this issue Feb 29, 2024 · 3 comments · Fixed by #1423
Closed
5 of 7 tasks

soroban network start followup improvements #1230

elizabethengelman opened this issue Feb 29, 2024 · 3 comments · Fixed by #1423
Assignees

Comments

@elizabethengelman
Copy link
Contributor

elizabethengelman commented Feb 29, 2024

The soroban network start and soroban network stop commands were added in PR # 1107, and as part of the PR discussion there were some follow up things that we wanted to improve on:

Todo

  • Add loading animation and some more information about what is happening as the docker container starts up (see feat: start network docker container with cli #1107) - we decided that mentioning to the user that network container logs exists is enough
  • Try several runtime defaults in parallel and return when one of them connects, see (feat: start network docker container with cli #1107) - we are already trying the most common default ("unix:///var/run/docker.sock" for UNIX & mac, and "npipe:////./pipe/docker_engine" for windows). if that doesn't work, which is common with docker desktop, we try "home/.docker/run/docker.sock". If that doesn't work, a user is still able to override the defaults with the docker_host flag
  • Does soroban network start make sense? Or should this eventually move to this to soroban instance start | stop or soroban network instance start | stop? See feat: start network docker container with cli #1107 (comment) for more info -> this was changed to soroban network container start
  • Better error handling when the container doesn't exist (specifically with the stop command)
  • Add a flag to change the name passed to docker

On the chopping block:

  • Add a healthcheck that makes sure that the docker container is up and running before releasing the user back to the command line (see feat: start network docker container with cli #1107) - Instead of a healthcheck, I've added the first few lines of the conatiner's logs into the command output, and also included a note about how to use stellar network container logs to see the rest of the logs. Is this good, or do we think that a health check is important?
  • support SSL connections - _I'm thinking that we could potentially skip this task. Since the quickstart is meant to be used for testing and development purposes, it seems okay to not have the connections be encrypted. created a new issue in case we run into this issue integration with laboratory
@elizabethengelman elizabethengelman converted this from a draft issue Feb 29, 2024
@leighmcculloch
Copy link
Member

leighmcculloch commented Mar 1, 2024

I think soroban instance start will serve us better, and be less confusing long term. Main motivation for separating it being that by bundling it into network we're making network do two mostly unrelated things:

  1. Configuring connecting to networks.
  2. Running networks / services.

While these things both involve networks, they're otherwise unrelated bundles of operations.

For example, take the following help output. It is easy to think that rm and stop both operate on local running networks, but actually rm removes configurations for the client. It is also easy to think that add might create a new network while start will boot it up, but actually add adds a configuration for the client and start is what creates a new network.

❯ soroban network -h
Configure networks

Usage: soroban network <COMMAND>

Commands:
  add    Add a new network
  rm     Remove a network
  start  Start a network
  stop   Stop a network
  ls     List networks

I think the relationship between the subcommands is much clearer if separated:

❯ soroban network -h
Configure networks for the client to connect to.

Usage: soroban instance <COMMAND>

Commands:
  add  Add a new network
  rm   Remove a network
  ls   List networks

❯ soroban instance -h
Run local instances of the Stellar network, RPC, and Horizon for local development and testing, as well as for joining testnet and pubnet networks.

Usage: soroban instance <COMMAND>

Commands:
  start Start a network
  stop Stop a network

@leighmcculloch
Copy link
Member

leighmcculloch commented Mar 1, 2024

Suggested follow up:

  • Auto-configure the network client for the running instance.

Edit: moved to own issue here:

@elizabethengelman elizabethengelman changed the title soroban network start followups soroban network start followup improvements Mar 5, 2024
@elizabethengelman elizabethengelman self-assigned this Mar 5, 2024
@janewang janewang moved this from Backlog to Todo in DevX Jun 18, 2024
@elizabethengelman elizabethengelman moved this from Todo to In Progress in DevX Jun 28, 2024
@janewang
Copy link
Contributor

janewang commented Jul 2, 2024

Add stellar network container logs command in the terminal and can close

@janewang janewang moved this from In Progress to Needs Review in DevX Jul 9, 2024
@github-project-automation github-project-automation bot moved this from Needs Review to Done in DevX Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants