[CLI] move out localnet logic + add command for Aptos Workspace #15508
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
⏱️ 3h 40m total CI duration on this PR
🚨 1 job on the last run was significantly faster/slower than expected
|
vgao1996
commented
Dec 5, 2024
Comment on lines
+1
to
+38
| // Copyright (c) Aptos Foundation | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| use crate::common::types::{CliCommand, CliResult, CliTypedResult}; | ||
| use async_trait::async_trait; | ||
| use clap::Parser; | ||
|
|
||
| /// Tool for operations related to Aptos Workspace | ||
| #[derive(Parser)] | ||
| pub enum WorkspaceTool { | ||
| Run(RunWorkspace), | ||
| } | ||
|
|
||
| impl WorkspaceTool { | ||
| pub async fn execute(self) -> CliResult { | ||
| use WorkspaceTool::*; | ||
|
|
||
| match self { | ||
| Run(cmd) => cmd.execute_serialized_without_logger().await, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[derive(Parser)] | ||
| pub struct RunWorkspace; | ||
|
|
||
| #[async_trait] | ||
| impl CliCommand<()> for RunWorkspace { | ||
| fn command_name(&self) -> &'static str { | ||
| "RunWorkspace" | ||
| } | ||
|
|
||
| async fn execute(self) -> CliTypedResult<()> { | ||
| aptos_workspace_server::run_all_services().await?; | ||
|
|
||
| Ok(()) | ||
| } | ||
| } |
There was a problem hiding this comment.
Note: this is the only new logic this PR adds. Everything else is just moving things around.
0xmaayan
approved these changes
Dec 5, 2024
| return Ok(()) | ||
| } | ||
| res = all_services_up => { | ||
| match res.context("one or more services failed to start") { |
There was a problem hiding this comment.
can we know what services?
There was a problem hiding this comment.
We are supposed to, but there's some funky stuff happening with error propagation.
My observation/theory is that when we have one service failing, other services that depend on it will likely short-cutting it thus preventing the original message from being shown. Let me double check the error propagation chain and make sure this gets fixed.
There was a problem hiding this comment.
Should be good enough for now, let's add a TODO to see if we can fix it later
Yes, I have another PR coming that will tackle this and a few other issues |
gregnazario
approved these changes
Dec 11, 2024
| return Ok(()) | ||
| } | ||
| res = all_services_up => { | ||
| match res.context("one or more services failed to start") { |
There was a problem hiding this comment.
Should be good enough for now, let's add a TODO to see if we can fix it later
vgao1996
force-pushed
the
cli-localnet-workspace
branch
from
f49d7ae to
e18b2fa
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
vgao1996
force-pushed
the
cli-localnet-workspace
branch
from
e18b2fa to
bceccd4
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
✅ Forge suite
|
jkjun4574
added a commit
to jkjun4574/aptos-core
that referenced
this pull request
Dec 12, 2024
…ce (aptos-labs#15508)" This reverts commit 803b7fd.
|
앱토스랩스 월렛 |
|
출금 |
|
/출금 |
Hi there, I'm jkj0585 👋Welcome to my GitHub profile! I'm passionate about software development and love to contribute to open-source projects. Here you'll find a collection of my work, projects, and contributions. 🔧 Technologies & Tools
📈 GitHub Stats🏆 Top Languages📫 How to reach me
🌟 ProjectsHere are a few projects I've worked on:
Feel free to explore my repositories and contribute to any projects that interest you! Thanks for visiting my profile! |
georgemitenkov
pushed a commit
that referenced
this pull request
Jan 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a new hidden command (
aptos workspace) to the CLI. Based on our previous discussion, this is the easiest way to distributeaptos-workspace-binary.To make this happen, I created a new crate
aptos-localnetand moved some of the shared logic there. This is to deal with circular dependencies. This also aligns with the long-term vision @banool and I landed on -- we'll unify the two localnet implementations and eventually move everything into theaptos-localnetcrate.