ErrorHandling: make GetComputationClient() return StatusOr<T> type.

[ysiraichi]

Ref: #9339

This PR creates a new version of GetComputationClient() that returns a StatusOr<T> type for better error handling. Since this function is frequently used in the codebase, its old version remains, but annotated with ABSL_DEPRECATED() (following #9381 example) and renamed to GetComputationClientOrDie(). The new version is created with the old name. Summarizing:

• Old GetComputationClient() is renamed to GetComputationClientOrDie()
• New torch_xla::runtime::status::GetComputationClient() function that returns StatusOr<T> type
• The old version remains, and calls the new function. It uses XLA_CHECK() for throwing an error on non-ok status, maintaining its old behavior
• New ConsumeAndMaybeThrow() function for throwing errors if non-ok status reach the Python API boundary

[ghpvnist]

Thanks for the change, added a few comments.

[ysiraichi]

Incorporated the changes suggested by @zhanyong-wan comment.

[ghpvnist]

Can you create a unit test for the new API? runtime_test.cpp in the same directory could work.

[ysiraichi]

@ghpvnist What kind of tests did you have in mind?
I think C++ tests usually go inside test/cpp directory.

[ghpvnist]

I think C++ tests usually go inside test/cpp directory.

This directory has precedent of tests within the same directory. I prefer to have tests close where the files are stored that way, future refactoring changes would be easier. Wdyt? @zhanyong-wan

What kind of tests did you have in mind?

I was thinking the typical unit tests for test coverage purposes. Something along the lines of:

• Check that calls to GetComputationClient return a valid client.
• GetComputationClientIfInitialized should return nullptr vs a valid client.
• etc.

[zhanyong-wan]

I think C++ tests usually go inside test/cpp directory.

This directory has precedent of tests within the same directory. I prefer to have tests close where the files are stored that way, future refactoring changes would be easier. Wdyt? @zhanyong-wan

While I prefer to have tests in the same directory as the code too, pytorch uses the test/cpp/test_foo.cpp convention. I think we should align with that to ease upstreaming.

[ysiraichi]

@zhanyong-wan @ghpvnist Let me know if there's anything else you'd like to see in this PR.

[zhanyong-wan]

Hi @ysiraichi , I'll take a look soon. In the future, when the PR is ready for another review, could you click on the spinning-arrows icon next to the reviewer name? That'll let us know that you are done with editing and it's our turn. Thanks!

[ysiraichi]

Ah, ok. Got it. I don't think I ever noticed that feature. Thanks for the tip.

[zhanyong-wan]

Thanks!