You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
"Default" options, note the quoting, are not truly defaults. Depending on where you are in the execution path they could mean two things:
if in resonate.run: The options used when registering a top level function
if in ctx.run: The options of the parent invocation
This "defaults" are then passed to the Invocation object just to be used in the split function which assigns "defaults" to some options when getting them from the argsOrOpts argument.
I think in neither case we should pass "default" opts to the invocation, we should always be in a position to pass a whole options object to the invocation and only use that one.
For ctx.run I think using the opts of the top level invocation as defaults of child invocations might be an undesirable pattern. for example, I can see how someone might want to set the timeout for a top level invocation very high but still expects a more sensible default timeout for the children invocations. With the current implementation all the children invocations will have the same timeout of the parent invocation.
Describe the solution you'd like
Remove the "default" options from the invocation object completely. This would simplify the invocation and its creation.
Use a proper default set of options when using ctx.run. This should be the sensible defaults that we use when registering a resonate function.
Rename the "default" options in resonate.run to something like registered options.
Refactor the split functions (plural intended) This is a major point of complexity and I think most of it could be avoided by keeping the split function a simple pure function that check if the last argument of a variadic set of arguments has the {_resonate: true} tag and returns {args, opts} defaulting to empty options. Each user of this function then handles the set of default options that wants to use.
Additional context
I am currently trying to find the right set of options to store as part of the DurablePromise, DeferredExecution and Top level executions might need to store some of the options in the durable promise, since the set of options that each invocation uses is tangled figuring out what to do, is complicated. I am trying to untangle the options story in the typescript sdk, so the options that we want to store become very clear.
The text was updated successfully, but these errors were encountered:
Mostly fixed at #125 with slight differences. Options still inherit from Resonate and the Registered options for the top level call of the current call graph.
Describe the problem you are facing
"Default" options, note the quoting, are not truly defaults. Depending on where you are in the execution path they could mean two things:
resonate.run
: The options used when registering a top level functionctx.run
: The options of the parent invocationThis "defaults" are then passed to the Invocation object just to be used in the
split
function which assigns "defaults" to some options when getting them from the argsOrOpts argument.I think in neither case we should pass "default" opts to the invocation, we should always be in a position to pass a whole options object to the invocation and only use that one.
For
ctx.run
I think using the opts of the top level invocation as defaults of child invocations might be an undesirable pattern. for example, I can see how someone might want to set the timeout for a top level invocation very high but still expects a more sensible default timeout for the children invocations. With the current implementation all the children invocations will have the same timeout of the parent invocation.Describe the solution you'd like
ctx.run
. This should be the sensible defaults that we use when registering a resonate function.resonate.run
to something like registered options.split
functions (plural intended) This is a major point of complexity and I think most of it could be avoided by keeping the split function a simple pure function that check if the last argument of a variadic set of arguments has the{_resonate: true}
tag and returns{args, opts}
defaulting to empty options. Each user of this function then handles the set of default options that wants to use.Additional context
I am currently trying to find the right set of options to store as part of the DurablePromise, DeferredExecution and Top level executions might need to store some of the options in the durable promise, since the set of options that each invocation uses is tangled figuring out what to do, is complicated. I am trying to untangle the options story in the typescript sdk, so the options that we want to store become very clear.
The text was updated successfully, but these errors were encountered: