Conversation
Signed-off-by: jackieismpc <jackieismpc@gmail.com>
Signed-off-by: jackieismpc <jackieismpc@gmail.com>
Signed-off-by: jackieismpc <jackieismpc@gmail.com>
Signed-off-by: jackieismpc <jackieismpc@gmail.com>
Signed-off-by: jackieismpc <jackieismpc@gmail.com>
Signed-off-by: jackieismpc <jackieismpc@gmail.com>
Signed-off-by: jackieismpc <jackieismpc@gmail.com>
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR implements Issue #541 by fully removing Dagrs’ synchronous execution/IO surface and migrating the project to an async-only API, updating examples/tests accordingly, and bumping the crate version to 0.8.0.
Changes:
- Remove legacy sync adapter APIs (e.g.,
start_with_runtime) and related error variants. - Remove blocking channel operations and migrate channel closing to async call sites.
- Convert tests/examples to
#[tokio::test]/#[tokio::main]and bump version to0.8.0.
Reviewed changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| project/dagrs/tests/v2_test.rs | Migrates graph execution tests to async #[tokio::test]. |
| project/dagrs/tests/typed_channel_test.rs | Migrates typed-channel tests to async entrypoints. |
| project/dagrs/tests/router_reuse_test.rs | Converts router reuse test to async execution/reset. |
| project/dagrs/tests/router_merge_test.rs | Converts router merge test to async execution. |
| project/dagrs/tests/loop_reset_test.rs | Converts loop reset test flow to async. |
| project/dagrs/tests/hook_test.rs | Migrates hook/retry tests to async and removes manual runtimes. |
| project/dagrs/tests/event_test.rs | Converts event broadcast tests to async and simplifies runtime usage. |
| project/dagrs/tests/checkpoint_test.rs | Converts checkpoint tests to async and removes runtime wrappers. |
| project/dagrs/tests/chain_skip_test.rs | Converts deadlock/timeout test to async tokio::time::timeout. |
| project/dagrs/tests/branch_pruning_test.rs | Converts pruning tests to async. |
| project/dagrs/src/utils/execstate.rs | Removes leftover sync-semaphore commented code. |
| project/dagrs/src/graph/mod.rs | Updates internal panic cleanup to await async channel-close; removes sync adapter + sync-context guard; updates internal tests to async. |
| project/dagrs/src/graph/error.rs | Removes BlockingCallInAsyncContext error variant. |
| project/dagrs/src/connection/out_channel.rs | Removes blocking send/broadcast APIs; makes close APIs async (now awaited by callers). |
| project/dagrs/src/connection/in_channel.rs | Removes blocking recv/map APIs; consolidates close API to async and introduces async close_all. |
| project/dagrs/examples/typed_action.rs | Migrates example to #[tokio::main] + async_start().await. |
| project/dagrs/examples/recv_any_example.rs | Migrates example to #[tokio::main] + async_start().await. |
| project/dagrs/examples/loop_dag.rs | Migrates example to async main; updates channel close calls to async. |
| project/dagrs/examples/hello_dagrs_async.rs | Removes redundant async hello example file. |
| project/dagrs/examples/hello_dagrs.rs | Migrates hello example to async main + async_start().await. |
| project/dagrs/examples/dagrs-sklearn/tests/dag_job_test.rs | Migrates sklearn example tests to #[tokio::test] and async execution. |
| project/dagrs/examples/dagrs-sklearn/src/parser/yaml_parser.rs | Small idiomatic cleanups (first(), avoid &p in get_mut). |
| project/dagrs/examples/dagrs-sklearn/src/parser/mod.rs | Removes unused import and simplifies ParseError conversion. |
| project/dagrs/examples/dagrs-sklearn/examples/sklearn.rs | Migrates sklearn example to async main; minor idiomatic indexing changes. |
| project/dagrs/examples/dagrs-sklearn/Cargo.toml | Adds tokio dependency for async macros in examples/tests. |
| project/dagrs/examples/custom_node.rs | Migrates example to async main + async_start().await. |
| project/dagrs/examples/conditional_node.rs | Migrates example to async main + async_start().await. |
| project/dagrs/examples/compute_dag.rs | Migrates example to async main + async_start().await. |
| project/dagrs/examples/auto_relay.rs | Migrates example to async main + async_start().await. |
| project/dagrs/docs/CHANGELOG.md | Adds 0.8.0 notes documenting async-only API + removals and migration guidance. |
| project/dagrs/README.md | Updates documentation to reflect async-only entrypoint and migration notes. |
| project/dagrs/Cargo.toml | Bumps version to 0.8.0; adds tokio macros feature for async test/main usage. |
| project/Cargo.lock | Updates lockfile reflecting new dagrs version. |
Comments suppressed due to low confidence (1)
project/dagrs/src/connection/out_channel.rs:135
TypedOutChannels::closeis markedasyncbut does not await anything. This can trigger Clippyunused_asyncand adds an unnecessary.awaitrequirement for callers. Either make it synchronous or introduce a real async operation (or explicitly allow the lint) if you need an async-only API surface.
/// Close the channel by the given `NodeId`, and remove the channel in this map.
pub async fn close(&mut self, id: &NodeId) {
if self.get(id).is_some() {
self.0.remove(id);
}
}
| /// Close the channel by the given `NodeId`, and remove the channel in this map. | ||
| pub fn close(&mut self, id: &NodeId) { | ||
| pub async fn close(&mut self, id: &NodeId) { | ||
| if self.get(id).is_some() { | ||
| self.0.remove(id); | ||
| } | ||
| } | ||
|
|
||
| pub(crate) fn close_all(&mut self) { | ||
| pub(crate) async fn close_all(&mut self) { | ||
| self.0.clear(); | ||
| } |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e376e1c3ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// Perform an asynchronous send on the outgoing channel for `NodeId`. | ||
| pub async fn send_to(&self, id: &NodeId, content: Content) -> Result<(), SendErr> { | ||
| match self.get(id) { | ||
| Some(channel) => channel.lock().await.send(content).await, |
There was a problem hiding this comment.
Keep a non-async send path for
LoopCondition
LoopCondition::should_continue is still a synchronous callback (project/dagrs/src/node/loop_node.rs:51), so custom loop controllers cannot switch to send_to(...).await or broadcast(...).await. Removing blocking_send_to here means any existing loop condition that emits a feedback signal or termination token through out now stops compiling with no replacement API; the same regression applies to the removed blocking broadcast helper in this impl.
Useful? React with 👍 / 👎.
#541
1.完全移除同步API以及相关代码
2.更新用例以及测试,全部使用异步入口
3.更新版本至0.8.0