documentation icon indicating copy to clipboard operation
documentation copied to clipboard

Side Effects and Mutable Side Effects

Open rachfop opened this issue 3 years ago • 8 comments

What does this PR do?

Side Effect

Adds Workflow Side Effect for:

  • [ ] Go
  • [ ] Java

Python and TypeScript do not currently support Side Effects.

https://github.com/temporalio/sdk-typescript/issues/144

Mutable Side Effect

Adds Mutable Side Effect for:

  • [ ] Go
  • [ ] Java
  • [ ] PHP
  • [ ] Python
  • [ ] TypeScript

Preview

NOTE

I didn't touch any of the random files in this PR.

It's probably from running yarn gen and Update branch.

Notes to reviewers

rachfop avatar Aug 24 '22 17:08 rachfop

Diagram of mutable side effects

rachfop avatar Aug 26 '22 17:08 rachfop

Not sure we should document this in the cross-language app dev guide until we're sure we're ever even going to have this feature across languages. If we choose not to have this feature across languages, we need to decide whether this is worth documenting at all and/or what the text would be saying why other languages don't have it.

cretz avatar Aug 29 '22 13:08 cretz

It does definitely need to be documented as a side effect is the only safe/correct way of working with external configuration in Workflow code. Using local activities for it is a hack, it's just a wrong abstraction. We should discuss if we should expose a safer interface and more user-friendly name over it. Like a "Marker". Because the Side Effect is a little technical and, actually, misleading.

Spikhalskiy avatar Aug 29 '22 15:08 Spikhalskiy

@Spikhalskiy - TS impl issue at https://github.com/temporalio/sdk-typescript/issues/144 is where we could potentially discuss different name or approach (no issue for Core or Python yet, it'd come from that). Then we can come back and update the doc stuff. Unsure if we need to leave this doc PR on hold until that is discussed.

cretz avatar Aug 29 '22 16:08 cretz

We have already decided that it is perfectly acceptable to document a feature that is not at parity with the other SDKs... We simply indicate that the feature is not currently available with that SDK.

flossypurse avatar Aug 30 '22 14:08 flossypurse

@flossypurse - :+1: My concern was whether this feature is important enough to document in the guide (regardless of whether it was important enough in the old guide). Note, the app guide doesn't document a dozen Go SDK features (await, selectors, channels, etc) and every SDK has many features we choose not to document there. If we feel side effects aren't deserving of this kind of documentation and may just add more noise than value, we can leave it up to the API documentation like we do with lots of other features.

cretz avatar Aug 30 '22 14:08 cretz

@cretz

Note, the app guide doesn't document a dozen Go SDK features (await, selectors, channels, etc)

Valid point - are these features covered by the sdk-features repo?

Are there other "concurrency" type of features in other SDKs that are somewhat on parallel? For example, is threading possible in Java, and do we expose a specific wrapper API to do that in Java like we do with Go channels?

@rachfop can you remind me if this is being tracked anywhere atm or if we made a decision about this yet?

flossypurse avatar Aug 30 '22 15:08 flossypurse

Valid point - are these features covered by the sdk-features repo? [...] Are there other "concurrency" type of features in other SDKs that are somewhat on parallel?

Not all of them are in sdk-features, or not yet. But for example, all SDKs provide a "wait for condition" type of API that we don't document. Same with timers/sleep (or at least I can't find the docs). Same for starting a coroutine.

Basically the actual API documents lots of things the docs doesn't (and probably shouldn't). Some are features all SDKs have, some are features only some SDKs have, some are features only one SDK has. The decision to document should just be based on importance vs noise.

For example, is threading possible in Java, and do we expose a specific wrapper API to do that in Java like we do with Go channels?

For channels, Java does have Workflow.newQueue which provides similar behavior to Go's workflow.NewBufferedChannel. For threading/coroutines, Java does have Async.function/Async.procedure which provides similar behavior to Go's workflow.Go.

I don't believe we document any of this (and again, maybe we shouldn't, but maybe we should, unsure).

cretz avatar Aug 30 '22 15:08 cretz