Skip to content

Conversation

@davidmrdavid
Copy link
Contributor

@davidmrdavid davidmrdavid commented Nov 19, 2021

Issue describing the changes in this PR

resolves #646
replaces #700

Pull request checklist

Additional information

Previously, the Function BindOutputFromResult would only bind output values for non-DF Function types. It appears this was put in place to prevent DF orchestrators from having multiple output bindings. However, the implementation also prevented Activity Functions from having multiple output bindings, which led to the bug report linked above.

This PR refines the logic to apply only to orchestrators.

@davidmrdavid
Copy link
Contributor Author

I'm unsure if I need to mark the checkbox to backport this PR, please let me know!

@michaelpeng36
Copy link
Contributor

I think this PR would be a good candidate for backporting due to the generic nature of the issue. However, for now, I think we should restrict this change to the v3.x/ps7, v4.x/ps7.0, and v4.x/ps7.2 branches since other branches do not have the same TODO comment.

@davidmrdavid
Copy link
Contributor Author

Thanks @michaelpeng36! I'd appreciate a review as well to move this along :)

@AnatoliB: do you agree with Michael's backporting versions suggestion? I want to make sure we target the version of the affected customer

Copy link
Contributor

@michaelpeng36 michaelpeng36 left a comment

Choose a reason for hiding this comment

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

LGTM

@Francisco-Gamino
Copy link
Contributor

Thanks @michaelpeng36! I'd appreciate a review as well to move this along :)

@AnatoliB: do you agree with Michael's backporting versions suggestion? I want to make sure we target the version of the affected customer

Thanks @davidmrdavid -- We should port this fix to v3.x/ps7 and v4.x/ps7.0 only (as a cherry picks). When we do new releases for v4.x/ps7.2, we will merge the dev branch into v4.x/ps7.2. Let me know if you have any other questions. Gracias!

@AnatoliB
Copy link
Contributor

Thanks @michaelpeng36! I'd appreciate a review as well to move this along :)
@AnatoliB: do you agree with Michael's backporting versions suggestion? I want to make sure we target the version of the affected customer

Thanks @davidmrdavid -- We should port this fix to v3.x/ps7 and v4.x/ps7.0 only (as a cherry picks). When we do new releases for v4.x/ps7.2, we will merge the dev branch into v4.x/ps7.2. Let me know if you have any other questions. Gracias!

Yes, these are the correct branches. v2 and ps6 branches don't support Durable officially, so this fix is not applicable there anyway. Muchas gracias!

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.

Push-OutputBinding not working in ActivityTrigger

5 participants