Skip to content

1848: refactor: make use of async_trait::async_trait consistent across project - #69

Open
martin-augment wants to merge 5 commits into
mainfrom
pr-1848-2026-06-09-10-56-58
Open

1848: refactor: make use of async_trait::async_trait consistent across project#69
martin-augment wants to merge 5 commits into
mainfrom
pr-1848-2026-06-09-10-56-58

Conversation

@martin-augment

Copy link
Copy Markdown
Owner

1848: To review by AI

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0bf38230-5580-4fa7-b113-a60c54f3c5a1

📥 Commits

Reviewing files that changed from the base of the PR and between 608b929 and 1558331.

📒 Files selected for processing (14)
  • .cursor/rules.md
  • .gemini/rules.md
  • AGENTS.md
  • CLAUDE.md
  • ballista/core/src/client_pool.rs
  • ballista/core/src/event_loop.rs
  • ballista/core/src/planner.rs
  • ballista/executor/src/client_pool.rs
  • ballista/executor/src/execution_engine.rs
  • ballista/scheduler/src/cluster/memory.rs
  • ballista/scheduler/src/cluster/mod.rs
  • ballista/scheduler/src/scheduler_server/query_stage_scheduler.rs
  • ballista/scheduler/src/test_utils.rs
  • docs/source/user-guide/extensions-example.md

Walkthrough

This PR contains two independent updates: first, it adds configuration constraints to Cursor, Gemini, and agent rule files prohibiting AI agents from linking to GitHub issues/PRs during code reviews and from reviewing other agents' configuration files. Second, it refactors async trait macro usage across ballista's core, executor, and scheduler modules by replacing use async_trait::async_trait; imports with fully-qualified #[async_trait::async_trait] attribute paths. One trait annotation in the scheduler module also changes from #[tonic::async_trait] to the new fully-qualified form. Documentation examples are updated to match the refactored pattern.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pr-1848-2026-06-09-10-56-58

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request replaces the async_trait macro imports and usages with their fully-qualified path async_trait::async_trait across various Ballista crates and documentation. It also adds AI agent configuration and rule files (.cursor/rules.md, AGENTS.md, and CLAUDE.md). There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@claude

claude Bot commented Jun 9, 2026

Copy link
Copy Markdown

Code Review

Overview

This PR enforces a consistent style for the async_trait macro across the project by replacing the two-step form:

use async_trait::async_trait;
#[async_trait]

with the fully-qualified, import-free form:

#[async_trait::async_trait]

The change touches 10 Rust source files and 1 documentation file. No logic is altered.


Code Quality & Style

  • Consistency: The migration is complete — a follow-up grep confirms no bare #[async_trait] attributes remain anywhere under ballista/. Good.
  • Fewer imports: Removing the use async_trait::async_trait; lines reduces per-file boilerplate and makes the crate origin of the attribute immediately visible at the use-site. This is the idiomatic Rust style for proc-macro crates that are used in only a handful of places per file.
  • tonic::async_traitasync_trait::async_trait (in ballista/scheduler/src/cluster/mod.rs): tonic re-exports async_trait unchanged, so this is semantically identical. Switching to the direct crate is preferable — it avoids depending on an implementation detail of tonic's public API that could silently disappear in a future tonic release.

Potential Issues

Spurious blank line in ballista/scheduler/src/test_utils.rs (line 27-28):
After the use async_trait::async_trait; import was removed, a double blank line was left between use std::time::Duration; and use crate::config::SchedulerConfig;. This diverges from the single-blank-line separator used everywhere else in the file. Worth cleaning up in a follow-up or squash.


Correctness

No behaviour changes. All modified sites are purely macro-attribute and import housekeeping. The underlying generated code from async_trait::async_trait is identical regardless of whether the macro is referenced through the crate path or a local use alias.


Test Coverage

No new tests are needed — this is a pure mechanical refactor with no logic change. The existing test suite covers the affected traits and impls.


Summary

Approve. The change is safe, the migration is complete, and the rationale (uniform style, explicit crate paths) is sound. The only actionable item is the double blank line in test_utils.rs, which can be fixed as a drive-by or in the next PR touching that file.

@augmentcode

augmentcode Bot commented Jun 9, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: Standardizes async-trait usage across Ballista by switching to the fully-qualified #[async_trait::async_trait] attribute.

Changes:

  • Removes per-file use async_trait::async_trait; imports and updates trait/impl annotations accordingly in core, executor, and scheduler crates
  • Replaces #[tonic::async_trait] with #[async_trait::async_trait] on scheduler state traits for consistency
  • Updates the user-guide extension planner example to match the new attribute form

Notes: This appears to be a mechanical refactor aimed at consistent macro qualification throughout the project.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

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.

3 participants