Skip to content

refactor(bindings/dotnet)!: remove redundant async overloads and simplify Operator - #8316

Open
Fatorin wants to merge 3 commits into
apache:mainfrom
Fatorin:dotnet-operator-cleanup
Open

Fatorin wants to merge 3 commits into
apache:mainfrom
Fatorin:dotnet-operator-cleanup

Conversation

@Fatorin

@Fatorin Fatorin commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

None.

Rationale for this change

Operator has async overloads that only omit the options argument.

They add API surface without adding capability, and every new operation would grow the same pair.

This PR removes them and keeps one method per operation; overloads are reserved for genuinely different inputs.

The same pass simplifies Operator construction and fixes a misnamed callback alias on the Rust side.

What changes are included in this PR?

  • Remove the CancellationToken-only overloads of ReadAsync, WriteAsync, StatAsync, and ListAsync. Tests pass the token by name instead.

  • Rename the Rust callback alias WriteCallback to VoidCallback; it reports success or failure only and is shared by every value-less operation.

  • Move native operator construction into a private static Construct helper so both public constructors are a guard clause followed by SetHandle, and drop the private parameterless constructor that existed only to seed a Lazy<OperatorInfo>. Info is now loaded eagerly into a readonly field during construction, matching the Java binding, which also removes the CA1419 suggestion on the SafeHandle subclass.

Are there any user-facing changes?

Yes. Four public overloads are removed (see below), and constructing an Operator now retrieves its OperatorInfo immediately instead of on first access to Info, so an info retrieval failure surfaces from the constructor rather than from the property. No documentation referenced the removed overloads.

Breaking changes

Affected packages: bindings/dotnet

Migration:

  • Replace op.ReadAsync(path, token) with op.ReadAsync(path, cancellationToken: token).
  • Replace op.WriteAsync(path, bytes, token) with op.WriteAsync(path, bytes, cancellationToken: token).
  • Replace op.StatAsync(path, token) with op.StatAsync(path, cancellationToken: token).
  • Replace op.ListAsync(path, token) with op.ListAsync(path, cancellationToken: token).

Calls that already pass options positionally, and calls that pass no token, are unaffected.

AI Usage Statement

  • Harness: Claude Code (desktop app)
  • Model: Claude Fable 5.1 (claude-fable-5-1)
  • Effort: high
  • Role: Drafted the code changes, the test call-site updates, and this description under the author's direction; the author chose which overloads to remove, the constructor and Info design, and the commit layout, and reviewed every change. Assumption affecting review: Info is not read on any internal code path, so eager loading adds one operator_info_get call per constructed operator, including operators created by Duplicate and WithLayer, and no per-operation cost.

@github-actions github-actions Bot added bindings/dotnet breaking-changes releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" size:L This PR changes 100-499 lines, ignoring generated files. labels Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bindings/dotnet breaking-changes releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant