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

[RFC] CLI #10

Merged
merged 80 commits into from Sep 26, 2023
Merged

[RFC] CLI #10

merged 80 commits into from Sep 26, 2023

Conversation

danielterwiel
Copy link
Contributor

@danielterwiel danielterwiel commented Sep 13, 2023

Usage:

Available commands inside the workspace:

  • cargo run: shows available CLI commands
  • cargo run build: builds project using xtask
  • cargo run start: starts dev server
  • cargo run stop: stops dev server
  • cargo run deploy: deploys component to path

@danielterwiel danielterwiel changed the base branch from main to dani_xtask September 13, 2023 13:40
@danielterwiel danielterwiel changed the base branch from dani_xtask to main September 13, 2023 15:59
@danielterwiel danielterwiel force-pushed the dani_clap_cli branch 3 times, most recently from 526a9e8 to 5d650e5 Compare September 14, 2023 15:48
cli/src/main.rs Outdated
println!("HTTP development server listening on {}", http_addr);
println!("RPC server listening on {}", rpc_addr);

let () = if !is_peer_connected("127.0.0.1:3001".parse().unwrap()).await {
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 what if we tried just polling the Echo endpoint of the RPC server a few times in a loop with a delay after each poll. Its very hacky - but it'll give us confirmation that the server is started.

cli/src/main.rs Outdated Show resolved Hide resolved
cli/src/main.rs Outdated Show resolved Hide resolved
@danielterwiel danielterwiel changed the title [WIP] CLI [RFC] CLI Sep 25, 2023
Copy link
Contributor

@SuddenlyHazel SuddenlyHazel left a comment

Choose a reason for hiding this comment

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

Nice work Dani!

Left a few comments, but overall this LGTM pending your final review :)

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
@@ -0,0 +1,29 @@
// FIXME: this is a copy of /development_server/proto/development.proto
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have to do anything here? Why did we have to copy this fella?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Quoting @SuddenlyHazel 😄:

it prob makes sense to eventualy get this into its own package.
But, for now feel free to just repeat the boilerplate in the CLI.

Update: Sharing .proto files across workspaces is harder than it seems. Wasted too much time on it past two days
Update 2: Managed to get it working 😅: b84f79e

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Next step would be to also move the entirety of tonic into its own package I assume?

development_server/src/rpc.rs Show resolved Hide resolved
cli/src/main.rs Outdated Show resolved Hide resolved

loop {
let rpc_addr = format!("http://{}:{}", domain, rpc_port);
let state = server_state(rpc_addr, just_started).await;
Copy link
Contributor

Choose a reason for hiding this comment

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

no-op here

Just bringing this to your attention for future Tokio hacking purposes

https://docs.rs/tokio/latest/tokio/time/fn.timeout.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wondering what you mean by no-op. When I println! the value of state it is in fact looping till the development_state is Echo'able:

     Running `target/debug/cli start`
[2023-09-26T13:27:46Z INFO  cli] Starting development server
[2023-09-26T13:27:46Z INFO  cli] Started development server
🪵 [main.rs:252]~ token ~ state = StartingUp
🪵 [main.rs:252]~ token ~ state = StartingUp
[2023-09-26T13:27:48Z WARN  cli] Development server already listening
🪵 [main.rs:252]~ token ~ state = Started
[2023-09-26T13:27:48Z INFO  cli] You can reach the development server on http://127.0.0.1:3001
[2023-09-26T13:27:48Z INFO  cli] handle_stderr:     Finished dev [unoptimized + debuginfo] target(s) in 0.33s

cli/src/main.rs Outdated Show resolved Hide resolved
cli/src/main.rs Show resolved Hide resolved
@SuddenlyHazel
Copy link
Contributor

Ship it 🥳 . This is looking great. I think we can move forward here :). We can always come back to clean things up in the future.

@danielterwiel danielterwiel merged commit ecbf672 into main Sep 26, 2023
@danielterwiel danielterwiel deleted the dani_clap_cli branch October 9, 2023 11:42
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.

None yet

3 participants