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

[FEATURE] Improve Workflow api #29

Closed
4 tasks done
dbwiddis opened this issue Sep 11, 2023 · 3 comments · Fixed by #26
Closed
4 tasks done

[FEATURE] Improve Workflow api #29

dbwiddis opened this issue Sep 11, 2023 · 3 comments · Fixed by #26
Assignees
Labels
enhancement New feature or request

Comments

@dbwiddis
Copy link
Member

dbwiddis commented Sep 11, 2023

Is your feature request related to a problem?

I implemented the Workflow API committed in #25 as part of my draft PR #26 and identified a few needed improvements.

What solution would you like?

Looking up and executing these should conceptually be similar to OpenSearch TransportActions. They all have an ActionType (fancy wrapper for a string), and then an action taking a Request argument and a Response type, which is used internally to return the results asynchronously. We need counterparts of these same 4 components here:

  • We need a consistent way to tie a string name to a Java object. Some sort of internal name field for each class will allow us to initialize a map somewhere and look these up.
  • Another <Workflow> is not the appropriate return type as all we can do with the returned future is execute it... we need two new interfaces here representing input and output. I think accepting input as an argument and a completable future for the output is probably the best way to do it.
    • I'm thinking a Map<String, Object> is probably a good internal representation for these input/output objects (to ease going to/from JSON) but there may be a better one. It's probably very analogous to XContent.
  • Do not require the execute() method to throw an Exception. This requires calling code to catch the exception. We're already returning a CompletableFuture, just complete the future exceptionally on error.
  • I'm not sure Workflow is the right class name as that seems to imply the highest level, while this corresponds either to individual process steps, or sub-workflows. I think WorkflowStep might be a good option but there may be better ones.

What alternatives have you considered?

I think these suggestions are open-ended enough to allow lots of alternatives, I'm just requesting functionality.

Do you have any additional context?

Somewhere we'll need a way of finding these java objects to iterate over them to get their names. This requires some level of object "registration" via an extension point somewhere. Handling via createComponents() is a possibility.

@owaiskazi19
Copy link
Member

@dbwiddis I have raised a draft PR to handle the above cases except the 2nd one. Can you elaborate more on the input and output method of the interface?
As any TransportAction extends HandledTransportAction which is an abstract class.
Are you suggesting to create 2 methods something like

Map<String, Object> input();

CompletableFuture<WorkflowStep> output();

@dbwiddis
Copy link
Member Author

dbwiddis commented Sep 12, 2023

@dbwiddis I have raised a draft PR to handle the above cases except the 2nd one. Can you elaborate more on the input and output method of the interface?

Without having looked at your PR yet, what I had in mind was something like this for the interface signature (please don't let this constrain what you do, haven't tried it, but think it might work):

CompletableFuture<WorkflowOutput> execute(WorkflowInput input);

And the WorkflowOutput / Input classes (or possibly just one class WorkflowData representing both) would have some internal representation of JSON (or XContent in general, thus the Map<String, Object> idea) with some helper methods for parsing and/or accessing fields, etc.

@dbwiddis
Copy link
Member Author

dbwiddis commented Sep 12, 2023

possibly just one class WorkflowData representing both

And since we might just feed one output to the input of the other this may be preferred. Object may not be the right type signature for an interface, might need ? or ? extends Something etc. because we will want our subclasses to have implementations of it.

So

CompletableFuture<WorkflowData> execute(@Nullable WorkflowData data);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants