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

fix orchestrator validator args #2185

Merged
merged 4 commits into from
Dec 8, 2023
Merged

fix orchestrator validator args #2185

merged 4 commits into from
Dec 8, 2023

Conversation

elliedavidson
Copy link
Member

@elliedavidson elliedavidson commented Dec 8, 2023

Closes #<ISSUE_NUMBER>

This PR:

  • Fixes a bug where the validators incorrectly connected to the orchestrator URL

This PR does not:

Key places to review:

orchestrator/src/client.rs

How to test this PR:

Run
cargo run --example orchestrator-webserver --features="libp2p/rsa " -- http://localhost:5555 ./crates/orchestrator/run-config.toml in one terminal and
cargo run --example multi-validator-webserver --features="libp2p/rsa" -- 10 http://localhost:5555 in another. It should correctly start consensus.

@rob-maron
Copy link
Collaborator

rob-maron commented Dec 8, 2023

#2168 changes this, but the examples aren't included in the basic lint/build anymore

Having a problem with some of the examples (like all-webserver), will add to this PR

args.url,
args.port
);
tracing::error!("connecting to orchestrator at {:?}:{:?}", args.url,);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
tracing::error!("connecting to orchestrator at {:?}:{:?}", args.url,);
tracing::error!("connecting to orchestrator at {:?}", args.url);

@elliedavidson
Copy link
Member Author

#2168 changes this, but the examples aren't included in the basic lint/build anymore

Having a problem with some of the examples (like all-webserver), will add to this PR

Ah yes, we also need to update those examples.

jbearer
jbearer previously approved these changes Dec 8, 2023
Copy link
Member

@jbearer jbearer left a comment

Choose a reason for hiding this comment

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

LGTM

@elliedavidson elliedavidson merged commit 4810b8e into main Dec 8, 2023
4 of 6 checks passed
@elliedavidson elliedavidson deleted the ed/fix-orch-url branch December 8, 2023 15:11
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.

4 participants