Skip to content

Add support for workflow output setting#67

Merged
ognjenkatic merged 6 commits into
masterfrom
feat/master/workflow-output
Nov 11, 2022
Merged

Add support for workflow output setting#67
ognjenkatic merged 6 commits into
masterfrom
feat/master/workflow-output

Conversation

@ognjenkatic

Copy link
Copy Markdown
Collaborator

This PR should allow setting workflow outputs by combining the outputs of various tasks and workflow inputs.

@ognjenkatic
ognjenkatic requested a review from boma96 October 21, 2022 08:49
@ognjenkatic ognjenkatic changed the title Add support for setting Add support for workflow output setting Oct 21, 2022
Expression<Func<TWorkflow, TerminateTaskInput>> input
) => AddAndReturnBuilder(new TerminateTaskBuilder(reference.Body, input.Body));

public void SetOutput<F>(Expression<Func<TWorkflow, F>> input) where F : WorkflowOutput

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this method be parameterized with the type already specified as wf output?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no point in specifying workflow output then.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TWorkflow is ITypedWorkflow here and there is no compile time relationship with the input and output parameters. But, I will move this PR back to draft and try to think of some better way.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, I did not notice that.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to introduce a constraint for TWorkflow to be of type Workflow<TInput, TOutput>. Technically that would break SemVer rules but in this particular case it is highly unlikely anyone will be affected.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of implemented this, but I am not really happy with how it works. Take a look

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ok, I think there is no other way of implementing this. I guess at some point we will drop this builder and use one with specific types but we will see.

@ognjenkatic
ognjenkatic marked this pull request as draft October 24, 2022 19:02
@ognjenkatic
ognjenkatic marked this pull request as ready for review November 3, 2022 20:31
@ognjenkatic
ognjenkatic requested a review from boma96 November 3, 2022 20:33
@ognjenkatic
ognjenkatic merged commit b8004a2 into master Nov 11, 2022
@ognjenkatic
ognjenkatic deleted the feat/master/workflow-output branch November 11, 2022 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants