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

Jump to bottom

Fix/network container improvements #1423

Merged

elizabethengelman merged 38 commits into stellar:main from AhaLabs:fix/network-container-improvements

Jul 22, 2024

Contributor

elizabethengelman commented Jul 1, 2024 •

edited

Loading

What

Improvements to docker network container commands. Closes #1230

Better error handling when the container doesn't exist (specifically with the stop command)
Add a flag to change the container name passed to docker

Why

To make stellar network container start/stop/logs a bit easier to use and to update.

Known limitations

I'm not sure that we have documentation about the network container commands at the moment, but it seems like it would be a helpful addition. stellar/stellar-docs#751

elizabethengelman force-pushed the fix/network-container-improvements branch 2 times, most recently from 98ad7fc to 6e3b601 Compare

July 3, 2024 19:24

elizabethengelman marked this pull request as ready for review

July 3, 2024 21:18

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/shared.rs Outdated Show resolved Hide resolved

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/shared.rs Outdated Show resolved Hide resolved

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/shared.rs Outdated Show resolved Hide resolved

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/start.rs Outdated Show resolved Hide resolved

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/start.rs Outdated Show resolved Hide resolved

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/stop.rs Outdated Show resolved Hide resolved

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/stop.rs Outdated Show resolved Hide resolved

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/logs.rs Outdated Show resolved Hide resolved

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/logs.rs Outdated Show resolved Hide resolved

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/start.rs Outdated Show resolved Hide resolved

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/start.rs Outdated Show resolved Hide resolved

willemneal requested changes

View reviewed changes

Member

willemneal left a comment

Overall looks great! Just some refactoring suggestions to clean it up and remove some duplicated code.

elizabethengelman added 8 commits

July 15, 2024 12:13


          Return a better error message when stopping container that doesn't exist

d26eb22


          Fix doc strings for network container commands

321a48f


          Pull shared container args to shared.rs

ae4ea7f


          Allow for overwriting the default container name with -c

5c67caf


          Check in generated doc updates

70b6c39


          Refactor printlns

89bceec


          Clippy & cleanup errors

d800be5


          Rename ContainerArgs -> Args

196c5e5

elizabethengelman force-pushed the fix/network-container-improvements branch from 517a7ac to 196c5e5 Compare

July 15, 2024 16:21

elizabethengelman and others added 4 commits

July 15, 2024 12:22


          Update cmd/soroban-cli/src/commands/network/container/start.rs

07ccbda

Co-authored-by: Willem Wyndham <[email protected]>


          Use expect instead of unwrap in get_container_name

523adb2


          Add get_additional_flags fn on Args to reduce duplication

c81f788


          Add get_container_name_arg fn on Args to reduce duplication

ce13f84

elizabethengelman requested a review from willemneal

July 15, 2024 16:38

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/shared.rs Outdated Show resolved Hide resolved

elizabethengelman force-pushed the fix/network-container-improvements branch from f42a9f9 to c2670e2 Compare

July 16, 2024 19:37

elizabethengelman added 3 commits

July 16, 2024 15:44


          Update help docs

a9fa5b0


          Clippy

aac2440


          Move connect_to_docker to Arg impl

3ed3586

elizabethengelman force-pushed the fix/network-container-improvements branch from 82c0a19 to 3ed3586 Compare

July 16, 2024 21:17

elizabethengelman added 11 commits

July 16, 2024 17:40


          Refactor start command: move helper fns to Cmd impl

fcb28dc


          Merge branch 'main' into fix/network-container-improvements

33b51bd


          Default container name to network name

e997527

instead of prepending it with
stellar. This will simplify logs and stop commands.


          Update stop and logs commands to accept the container name

aadfc63

instead of the network.


          Update docs

e96b8e1


          Merge branch 'main' into fix/network-container-improvements

a18ac18


          Merge branch 'main' into fix/network-container-improvements

f79b59d


          Create a Name type to handle internal vs external name conventions

0c95562


          Prefix container names with 'stellar-'

235f521


          Add tests for Name struct

03d29ac


          Refactor Name fns

392e1f6

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/shared.rs Outdated Show resolved Hide resolved

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/start.rs Outdated Show resolved Hide resolved

leighmcculloch reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/shared.rs Outdated Show resolved Hide resolved

willemneal reviewed

View reviewed changes

cmd/soroban-cli/src/commands/network/container/shared.rs Show resolved Hide resolved

elizabethengelman and others added 3 commits

July 18, 2024 11:03


          Merge branch 'main' into fix/network-container-improvements

42a9bfc


          Merge branch 'main' into fix/network-container-improvements

6fbb3f3


          Update Name::new to just take in one arg

ec3d39f

Co-authored-by: Willem Wyndham <[email protected]>

elizabethengelman force-pushed the fix/network-container-improvements branch from 3719f91 to ec3d39f Compare

July 18, 2024 19:19

elizabethengelman added 2 commits

July 18, 2024 15:30


          Refactor Name

996dea4


          Merge branch 'main' into fix/network-container-improvements

c7c465c

elizabethengelman requested a review from willemneal

July 22, 2024 13:13

willemneal approved these changes

View reviewed changes

elizabethengelman merged commit 3fbf979 into stellar:main

23 checks passed

elizabethengelman deleted the fix/network-container-improvements branch

July 22, 2024 20:18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet