Closed cdavernas closed 3 months ago
In your example where is the refName: foo.greet function definition? is the function def named "greet" and you append the namespace to it?
@tsurdilo yes, exactly: {namespace}.{functionName}
Thanks, yeah i put a comment on that associated issue..do really like it but then why not
{namespace}.{stateName}
{namespace}.{actionName}
{namespace}.{eventDefName}
...
I think these type of changes are awesome but in the grand schema of things if added should be thought to be added to entire spec not just one piece at a time imo. we could find by looking at this on higher level more use to it than just function def and then realize its powerful and come up with approach so this can be added to everything
Events, functions, authentications, retries and timeouts. For states, that makes less sense imho considering the ability to call subflows. So only those should be cobsidered "resources" imo
Yeah I understand, i think with this also comes a lot of possible issues on user level (possible typos since you have to define a namespace in each action, maybe a better approach would be to have a top-level namespace property instead and if user does not set it would default to "default" which is the base resource def name.
One more question, does this apply only for "rest" action type? For everything else the resource / uri would be defined inside the offloaded json/yaml (openapi/asyncapi/grpc, etc)
If so I am very much not "game" with adding all this just for rest type as we as spec do prefer users to use the standards and not "rest"..I thought we adding rest only for small scale scenarios and tests for most part.
No it should be for everything imo. There is no difference anyways.
Ok so can we see example with openapi where the uri is defined in the openapi def? I must be missing something
You can define a grpc function def with authRef
It's defined in the external resource, as you would right now. It's just moved out the wf doc and into the external res, possibly with its referenced auths
Ah ok so here we are just talking about the actual uri resource to the openapi definition ok ok.
Yeah i think let's maybe look at having a "resourcesNS" top level property instead than having the do the weird lookup of each function def by appending namespace pls, think that would make it easier on users.
The whole point is to be able to have multiple namespaces, so a top level ns property is not gonna help, or I'm not getting what you mean.
Yes multiple namespaces but a single one would apply to the whole workflow definition when set, so you can have a wf exec in test env using ns A and in staging B if you wanted.
Also think that this can be implemented as an extension point, this is quite a big change imo to enforce namespace lookups from what we have now and if impls wanna use it just define extension point for it and set the mapping of like stateName->actionName to namespace there
I think often we forget that our extension mechanism is quite powerful in the spec :) Either way if we go with this lets make sure it applies to all "resources" and to not enforce user to define a namespace on each of the reference point in their wf def but allow it to be set globally, would make things easier imo
No, it's meant to import multiple namespaces at the same time. For instance, myGreetingApp's and myLoggingApp's. And that has imo nothing to do with extensions, namespace is not the feature here. The feature is:
Plus, extension is what it is: duct and tape. Makes your definitions suddenly non portable, thus going against SW core principles
I agree with @cdavernas. This proposal deals with the discussion in #676 for all external resources we might have, adding the possibility to reuse the resources across the workflow definitions. Additionally, I'd say that if we make this move, it should be before 1.0.
It's a huge change but, at the same time, won't break anything since old definitions would be assumed to be in the default
namespace. This change helps more complex use cases or environments with multiple definitions that can be reused among other definitions.
@cdavernas Thanks for the proposal. I think the namespace separator should be : not . (basically because : is less likely to be part of a function name than .) And thinking about making implementors life easier, probably we should add resource type to the import. This has the benefit of restricting (and setting) the resources that can be imported (related with one of the aspect of the the conversation you are having with Tiho)
I think the namespace separator should be : not . (basically because : is less likely to be part of a function name than .)
@fjtirado Hmmm, not fan of the : though. First because the dot is commonly used for namespaces, at least in Borland. Second because I associate that char as the version separator, which is usually its purpose. But that's just a matter of personal preference I guess.
And thinking about making implementors life easier, probably we should add resource type to the import.
Very good point!
extensions do not go "against" anything , they are part of spec. we seem to be moving in direction of "spec for specific runtimes" than "spec for all runtimes" and id like not to move in that direction. if you guys feel otherwise pls ping me we can discuss i guess
@tsurdilo I disagree TO. I believe we are actually just moving from a theoretical spec to an implemented one. During implementation, I believe we discover limitations/issues otherwise unseen. Not addressing those limitations/issues, especially unanimous ones, is gonna ensure that people are gonna move away or branching the spec out imho.
Anyways, this has little to do with runtime, it's a pragmatic, useless limitation of the spec to only be able to import one external file per functions, per example. It makes external references useless, dot, except for very basic demo use cases.
This proposal addresses those changes, makes it easier to implement the spec (for newcomers: we already did it) and allows complex external files with embedded auth, etc. What's not to like about it?
well as author of this spec i respectfully can say we have different opinions on this
As another author (and owner) of the spec, I'd like you to respectfully elaborate:
Well, to me it's a no to all answers, and that's the motivation of this suggestion as well as of @fjtirado and of @ricardozanini ones.
Now, while I perfectly understand you might not like the proposed changes (tastes and colors, after all), just please acknowledge those issues (and trust us on this), work your magic out and propose unanimous (non-extension/non-metadata based) alternatives ❤️!
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Closed as resolved by 1.0.0-alpha1, and therefore as part of #843
What would you like to be added:
Refactor external references.
What I propose is:
resource
object, such as the following, used to encapsulate external definitions:In other words, an external function definition file would not contain an array of function definitions, but an object such as the above.
In addition, that would allow serverless-workflow compatible apps to provide a self-describing document making it ready to use in Serverless Workflow.
functions
,events
, etc. be of typeoneOf<uri, functionArray>
, they should be only of typearray
, as discussed in the meeting of the 27th of September, and in https://github.com/serverlessworkflow/specification/issues/676. A newimports
(orresources
) top-level properties would replace the need for uri-values, and would allow the import of multiple files instead of just one.Why is this needed:
Currently, external references are easy and simple to use, but unhappilly do not address all common/reasonnable use cases, as explained in https://github.com/serverlessworkflow/specification/issues/676. In addition, it forbids referencing in a safe manner external functions that define an
authRef