Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function definition namespaces (Importing several function definition files) #676

Closed
fjtirado opened this issue Sep 12, 2022 · 32 comments
Closed
Assignees
Labels
area: spec Changes in the Specification change: feature New feature or request. Impacts in a minor version change
Milestone

Comments

@fjtirado
Copy link
Collaborator

fjtirado commented Sep 12, 2022

As per spec "The functions property can be either an in-line function definition array, or an URI reference to a resource containing an array of functions definition. Referenced resource can be used by multiple workflow definitions."

This covers the following scenarios:

  • all function definitions to be used by an specific workflow are defined in the external file
  • all function definitions to be used are defined by the workflow itself.

But it does not cover two different scenarios that might arise:

  • using function definitions defined in multiple external files. Think of a file defining all functions for a particular api, another file defining all functions for another api and a workflow that orchestate the integration between these two APIS. Ideally you do not want to create a third file which merge the two others.
  • using an external file and adding specific functions for that particular workflow that wont be used in any other workflow. Ideally you do not want to update the external file with the private function

Therefore, I propose to update the Workflow specification to:

  • Establish that functions property can be only an array of function definitions (where all functions privative to that workflow are defined), but not an external reference.
  • Add a new property "namepaces" or "imports". That property will be an associative map, which value is the uri of a file containing array of function definitions and key is the namespace that identifies it within this workflow. The namespace is needed to avoid name clashing between different imported files. When calling the imported function within an state, the refName must have the namepace prefixed.

So, we will have something like in the header of the flow

"namespaces": {"myRpcAPI":"ARPCApi.json", "myOpenAPI": "OpenApi.json},
"functions": [{"name":"doSomething", "type": "expression", "operation":"jq  expression specific to this flow"}]

and

"refName": "myRpcAPI:doSomething"
"refName": "myOpenAPI:doSomething"
"refName":"doSomething"
@cdavernas
Copy link
Member

cdavernas commented Sep 12, 2022

@fjtirado Funny, I had the exact same proposal, as I started coding Synapse's resource server at the end of last week. Wanted to bring it up in today's meeting, but there you go! +1000

For namespaces, however, I'd try to find, if possible, something more elegant than file-based. And IMO the term is not ubiquitous enough, as the same concept could very well be applied to at least events and auth too. I have nothing in mind right now, but even something as functionNamespaces would be a better fit IMO.

@ricardozanini
Copy link
Member

I liked the idea as well, and I agree that namespaces sounds too generic. Not sure even about functionNamespaces, but it seems better.

I'd rather use import somehow since the <namespace>: implies that is a namespace already.

Maybe functionImports?

@fjtirado
Copy link
Collaborator Author

yes, I think functionImports is fine, I just mention namespace becuase of my c++ heritage ;)

@ricardozanini
Copy link
Member

ricardozanini commented Sep 12, 2022

Then:

{
    "functions": [{ 
         "operation": "myfile.yaml#myoperation"
         "name": "privateFunction1"
    }],
    "functionImports": {
         "sharedApi": "sharedFunctionDefinitions.json",
         "sharedApi2": "sharedFunctionDefinitions2.json"
    }
}

The format is "namespace": "reference".

Then in the states we refer by sharedApi:publicFunction1.

@ricardozanini ricardozanini added change: feature New feature or request. Impacts in a minor version change area: spec Changes in the Specification schema labels Sep 12, 2022
@ricardozanini ricardozanini added this to the v0.9 milestone Sep 12, 2022
@fjtirado
Copy link
Collaborator Author

@ricardozanini functionImports should be an object (it is a dictionary) not an array.

@tsurdilo
Copy link
Contributor

imo the aggregation should happen on runtime side. having an uri is enough for me. i rather not add this is possible, so sorry

@fjtirado
Copy link
Collaborator Author

@tsurdilo how do we agregate on runtime? can you give an example?

@tsurdilo
Copy link
Contributor

tsurdilo commented Sep 12, 2022

  "functions": "htttp/mycomp.org/my/prod/function/definitions?namespace=abc"

this uri endpoint can aggregate all your function defs and stick in the functions array which then is also validatable against our single functions schema

@cdavernas
Copy link
Member

cdavernas commented Sep 12, 2022

this uri endpoint can aggregate all your function defs and stick in the functions array which then is also validatable against our single functions schema

@tsurdilo IMHO, your suggestion is impossible to define and/or implement. How can you, as a runtime, determine the format of the data returned by a random url? Sometimes it might just be a JSON of a functionDef collection, sometimes a collection of collections, etc.

@tsurdilo
Copy link
Contributor

tsurdilo commented Sep 12, 2022

this is a compile time resolve so yes you can validate its response json, otherwise implement the validation on service side that returns the resolved json, thats fine too since in most cases you control it

@fjtirado
Copy link
Collaborator Author

fjtirado commented Sep 12, 2022

how does that URI endpoint knows what needs to be aggregated?
Remember, the use case is, the writer of the flow has already written a function definition array (lets call it file 1) for API 1 and another function definition array (lets call it file 2) for API 2, and it is already using file 1 for flows that interacts just with api 1 and file 2 for flows that interacts with api 2
now he wants to write a flow that interacts both with API 1 and API 2, how he can make a reference to both arrays with just one URI?

@tsurdilo
Copy link
Contributor

what we did this for (ability to use uri) is for runtimes to be able to provide function definitions out of the box to users.
in case where your users define these function defs they should figure that out on their own

@tsurdilo
Copy link
Contributor

these types of requests where its more about runtime user support and not the spec itself are things that all i can say is "deal with it", again sry.

@cdavernas
Copy link
Member

this is a compile time resolve so yes you can validate its response json, otherwise implement the validation on service side that returns the resolved json, thats fine too since in most cases you control it

No, you can't. First of all, there is no such thing as compile time. Second, you don't know what a endpoint you have no control on as implementer will return. As I said, it could be a single collection, a collection of collections, a file containing references to other files, etc. What you suggest will result in implementation mess and zero portability of SW between runtimes, even though that is the main focus of the spec.

@fjtirado
Copy link
Collaborator Author

fjtirado commented Sep 12, 2022

ok, scenario two, I have a external function defintion magic array provider (if I understood correctly) and now I want to define my own private array of function for that specific file (because my file use functions in that external URI and also expressions that are defined in that functions array) , how do I do that if functions property either points to the the URI or to the array of function definitions?

@tsurdilo
Copy link
Contributor

tsurdilo commented Sep 12, 2022

put it inline in the definition then. i honestly dont see anything that we can do except add some "improvements" that again wont work for everyone. so uri is uri

@fjtirado
Copy link
Collaborator Author

fjtirado commented Sep 12, 2022

ok, so you are basically proposing to copy paste the content of the function definition array contained in the file and add the private function inline, do you?

@tsurdilo
Copy link
Contributor

use expression if you want as the string and have runtime eval it if that makes it easier, idk. but we are starting to look like google cloud workflow here..and i dont like it

@fjtirado
Copy link
Collaborator Author

@tsurdilo uri is uri, but it can be uris, I honestly do not see the rationale agains that, I think you need to elaborate a bit more.

@cdavernas
Copy link
Member

we are starting to look like google cloud workflow here

On that specific subject, that's not necessarily a bad thing! Even though I'm sure we can perfectly work without that "feature", I'm convinced it's a good, quality of life one. Like the bookmarking we spoke about.

I believe the main problem here is: how to reference multiple (possibly namespaced) function (or event or auth, ...) definition collections in a PORTABLE way (meaning works the same accross implementations)? So far, we cant achieve that out of the box with what the spec proposes.

@tsurdilo
Copy link
Contributor

tsurdilo commented Sep 12, 2022

ok well if you guys feel this is helpful then go for it 👍
i have 0 clue how this adds to portability tho, it just adds more enforcement from spec side as to what users and runtimes can or cannot do, things that you can implement on your runtime side as well. i dont think there is anyone actually trying out the portability aspect atm so ok not sure. yeah go for it if you feel very strongly about it.

@fjtirado
Copy link
Collaborator Author

fjtirado commented Sep 12, 2022

ok, should we proceed to open a PR then?

@tsurdilo
Copy link
Contributor

ok i think i understand this better now,
I do like the namespace use but imo its a concept that if we add is something that could be applied to more than just function definitions.

imo this concept is great but would require some thinking how it would fit overall to the concept of all our dsl not just particular piece of function defs

imo right now this is already possible via metadata so lets think about this some more please

@github-actions
Copy link

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.

@github-actions
Copy link

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.

@github-actions
Copy link

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.

@github-actions
Copy link

github-actions bot commented Apr 4, 2023

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.

@cdavernas
Copy link
Member

@tsurdilo This cannot be achieved using metadata or extension without loss of intercompatibility , which is IMHO the main and most important aspect of a proper DSL.

@ricardozanini
Copy link
Member

Related to #691

@github-actions
Copy link

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.

@fjtirado
Copy link
Collaborator Author

fjtirado commented Feb 21, 2024

I think @cdavernas is covering this issue with #701, therefore closing

@github-project-automation github-project-automation bot moved this from Todo to Done in Progress Tracker Feb 21, 2024
@fjtirado
Copy link
Collaborator Author

fjtirado commented Feb 21, 2024

Superseded by #691

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: spec Changes in the Specification change: feature New feature or request. Impacts in a minor version change
Projects
Status: Done
Development

No branches or pull requests

4 participants