Enhancements to the Early Return sample

[steveandroulakis]

What was changed

The Early Return sample in core/src/main/java/io/temporal/samples/earlyreturn

• Better error handling in the client (to log "io.grpc.StatusRuntimeException: PERMISSION_DENIED: ExecuteMultiOperation API is disabled on this namespace.")
• Code comments in the workflow
• Added a link to the sample in the main README

Why?

Less failure-prone and easier to understand Early Return sample.

Checklist

1. Closes

n/a

2. How was this tested:

Existing tests still apply

3. Any docs updates needed?

No

[tsurdilo]

in description you mention specific grpc exception (StatusRuntimeException)
here just general Exception is asumed its of that type. could add some type check and maybe show how different exceptions could be handled differently in client

[steveandroulakis]

@tsurdilo I have provided a check for StatusRuntimeException and "ExecuteMultiOperation API is disabled" specifically (with advice). Please let me know if this is enough.

[dandavison]

I think that the early return update should return a distinct result type from the workflow. In my samples I've been imagining that the early return type is a "confirmation" or "validation" of the transaction, whereas the final workflow result would be some sort of statement of the final outcome of the transaction (I'm not an expert in this domain, but perhaps the exact amount transferred is not known until a later stage, e.g. due to market fluctuations/exchange rates etc)

[steveandroulakis]

This PR is for some logging enhancements to an already-accepted sample. It sounds like we're going back to the drawing board somewhat on what the sample should do?

For clarity, currently the outcome of the early-return is for example:

      {
        "transactionId": "TXID8524360451",
        "status": "Initialization successful"
      }

(the update mints a transaction id such as TXID8524360451)

And the workflow result is:

{
  "transactionId": "TXID8524360451",
  "status": "Transaction completed successfully."
}

@dandavison Please tell me how do better express the 'validation' vs 'final confirmation' step and I will look at making the changes

[dandavison]

We can leave them until a subsequent PR; we may still be making some adjustments to the API. I think one underlying issue is that the Go sample doesn't have a return type for the update. So yes, I think it would be clearest for users if the update and the workflow return distinct types, e.g. the update should return TransactionConfirmation(transactionId, confirmationCode, ...) and the workflow should return TransactionStatement(transactionId, finalAmount, ...). But yes, don't worry, I can make the changes, and I'll talk to @drewhoskins-temporal to de-naiveify the financial concepts.

[steveandroulakis]

Edit, made this reply before seeing the one you sent above, Dan.

It sounds like if I introduce a new object e.g. TxInitiationResult and make it return some stuff that feels very "this has been successfully initialized!" and keep the TxResult object for the workflow result that would satisfy.

I want to confirm this though, as I'm re-hashing a sample that was already accepted as-is and I'm trying to avoid endless churn on this. Thanks

[dandavison]

That sounds good. I'll get confirmation and post back here in an hour or less.

[dandavison]

Confirmed, @drewhoskins-temporal and I agree that distinct return types are desirable. But like I say, if you want to leave it then I can do it as I suspect there's going to be a tweak to the API coming soon.