Skip to content

Error Handling: refactor the existing ComputationClient implementations to use status QOL functions.#9420

Merged
ysiraichi merged 3 commits intomasterfrom
ysiraichi/status-for-computation-clients
Jul 16, 2025
Merged

Error Handling: refactor the existing ComputationClient implementations to use status QOL functions.#9420
ysiraichi merged 3 commits intomasterfrom
ysiraichi/status-for-computation-clients

Conversation

@ysiraichi
Copy link
Copy Markdown
Collaborator

@ysiraichi ysiraichi commented Jun 28, 2025

This PR modifies IfrtComputationClient and PjRtComputationClient in a similar way that XlaCoordinator was modified in #9386, by:

  • Creating a private struct PrivateUse for each of them, so as to make their constructor callable only in private contexts
  • Creating a Create() static member function that actually instantiates them, calling their constructor
  • Creating an Initialize() member function that is called by Create(), so as to initialize the class' members
  • Making these new member functions handle status types, checking and propagating them

@ysiraichi ysiraichi changed the title Refactor computation clients to use factory pattern Error Handling: refactor the existing ComputationClient implementations to use status QOL functions. Jun 28, 2025
@ysiraichi
Copy link
Copy Markdown
Collaborator Author

ysiraichi commented Jun 28, 2025

Blocked until #9419 is merged.

@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-computation-clients branch from 328870d to 5ef2bb3 Compare June 28, 2025 14:41
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-pjrt-registry branch from 50d9c5a to e1bf39d Compare July 1, 2025 10:55
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-computation-clients branch from 5ef2bb3 to 4873c0d Compare July 1, 2025 10:55
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-pjrt-registry branch from e1bf39d to 2083d31 Compare July 1, 2025 15:09
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-computation-clients branch from 4873c0d to ee438d0 Compare July 1, 2025 15:10
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-pjrt-registry branch from 2083d31 to 40db714 Compare July 1, 2025 16:06
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-computation-clients branch from ee438d0 to fc8dcd5 Compare July 1, 2025 16:07
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-pjrt-registry branch from 40db714 to 9ebf41c Compare July 1, 2025 16:28
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-computation-clients branch from fc8dcd5 to 23263bf Compare July 1, 2025 16:28
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-pjrt-registry branch from 9ebf41c to 8499caf Compare July 1, 2025 18:14
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-computation-clients branch from 23263bf to 8ba721e Compare July 1, 2025 18:14
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-pjrt-registry branch from a4b9bd4 to c0e7506 Compare July 2, 2025 18:26
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-computation-clients branch from 8ba721e to df432d2 Compare July 2, 2025 18:27
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-pjrt-registry branch 3 times, most recently from c4595dc to 3c1289f Compare July 10, 2025 12:06
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-computation-clients branch from df432d2 to 9c19b16 Compare July 10, 2025 19:05
@ysiraichi ysiraichi changed the base branch from ysiraichi/status-for-pjrt-registry to master July 10, 2025 19:05
@ysiraichi ysiraichi marked this pull request as ready for review July 15, 2025 13:25
zhanyong-wan
zhanyong-wan approved these changes Jul 15, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@zhanyong-wan zhanyong-wan left a comment

Choose a reason for hiding this comment

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

Nice! Thank you!

Comment thread torch_xla/csrc/runtime/runtime.cpp
ysiraichi added 3 commits July 16, 2025 14:25
@ysiraichi
Refactor computation clients to use factory pattern
d5cd175 
Update IFRT and PjRt computation clients to use `Create()` factory
methods:
- Replace constructors with factory methods that return `StatusOr<T>`
- Use `ConsumeAndMaybeThrow` for `XlaCoordinator::Create` integration
- Improved error handling with proper status propagation
@ysiraichi
Last touches.
5581464
@ysiraichi
Tweak.
be08367
@ysiraichi ysiraichi force-pushed the ysiraichi/status-for-computation-clients branch from 0af57e2 to be08367 Compare July 16, 2025 17:25
@ysiraichi
Copy link
Copy Markdown
Collaborator Author

ysiraichi commented Jul 16, 2025

I will land this for now.

@zhanyong-wan
We can get back to it if you think we should return a unique_ptr<T> there (see comment).

@ysiraichi ysiraichi merged commit 866fd22 into master Jul 16, 2025
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zhanyong-wan zhanyong-wan zhanyong-wan approved these changes

@ghpvnist ghpvnist Awaiting requested review from ghpvnist

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ysiraichi @zhanyong-wan