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

feat: multi-validator bash demo #13

Merged
merged 13 commits into from
Oct 29, 2024
Merged

feat: multi-validator bash demo #13

merged 13 commits into from
Oct 29, 2024

Conversation

agouin
Copy link
Member

@agouin agouin commented Oct 29, 2024

Update README hello world demo to use four validators with storage local in the test/ directory for config and log browsing during test execution.

Perform genesis assembly in isolated working dir, then distribute out to all vals.

Ctrl+C shuts down all processes and cleans test files.

Resolves #3

Copy link
Member

@mark-rushakoff mark-rushakoff left a comment

Choose a reason for hiding this comment

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

You should run this through shellcheck. I think it's going to complain about most variables being unquoted, not sure if it will have any other issues.

The shell script looks fine aside from probably not shutting down the other validators, but we can fix that later.

I agree that starting the 4 validator case by default is better for demo purposes, but it probably makes sense to still have a make target to start a single validator network, as that is much easier for debugging known issues.

Raising the signal handling up to main.go is the only part that would be better to change now than later, I think.

.gitignore Show resolved Hide resolved
@@ -73,6 +74,14 @@ func newSeedCommand() *cobra.Command {

fmt.Fprintf(cmd.ErrOrStderr(), "Seed running at multiaddrs: %s\n", joinedAddrs)

c := make(chan os.Signal, 1)
Copy link
Member

Choose a reason for hiding this comment

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

Could you raise this section up to

gcosmos/main.go

Lines 27 to 28 in 629f0ff

// We should probably use an os.SignalContext for this in order to ^c properly.
context.Background(),
so that all commands inherit interrupted contexts? And use signal.NotifyContext, which actually exists, unlike the imaginary type I put in the linked comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

srvcmd.Execute builds it's own context.Background(), so had to copy the Execute function into main.go and make it take a context.

scripts/run_gcosmos.sh Outdated Show resolved Hide resolved
./gcosmos gordian seed ./test/p2p.seed.txt &

# Wait for the seed node to start
sleep 2
Copy link
Member

Choose a reason for hiding this comment

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

We could add a command to block until the seed server is ready, so you don't have to wait extra time if it starts quickly. That is complex enough to warrant a separate PR.

scripts/run_gcosmos.sh Show resolved Hide resolved
@agouin agouin merged commit 9b73568 into main Oct 29, 2024
2 checks passed
@agouin agouin deleted the andrew/multi_val_demo branch October 29, 2024 15:57
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.

Launch Demo Readiness
2 participants