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

[Revert ]Merge Actor activation and first method/reminder call to user service. #219

Closed
amanbha opened this issue Aug 15, 2019 · 8 comments · Fixed by #280
Closed

[Revert ]Merge Actor activation and first method/reminder call to user service. #219

amanbha opened this issue Aug 15, 2019 · 8 comments · Fixed by #280
Assignees
Labels

Comments

@amanbha
Copy link
Contributor

amanbha commented Aug 15, 2019

Currently 2 calls happen into user code when a method/reminder is invoked on an inactive actor:

  1. Actor is activated.
  2. Method/reminder is invoked.

Instead just invoke the ActorMethod and provide an optional header (or query param) to indicate activation before method invocation. SDK/User code can look at this header(query param) to determine whether it needs to be activated.

@yaron2
Copy link
Member

yaron2 commented Aug 15, 2019

I generally support that notion, however I don't like the use of headers unless absolutely critical.
Thoughts?

@amanbha
Copy link
Contributor Author

amanbha commented Aug 15, 2019

A decision has to be made by SDK/UserCode, so a flag indicating ActivateBeforeInvocation needs to be passed from Actions runtime. It can either be a query parameter or a header.

@yaron2
Copy link
Member

yaron2 commented Aug 15, 2019

query param then.

@yaron2
Copy link
Member

yaron2 commented Aug 15, 2019

We need to open an issue for this in the spec repo to update the API endpoint, but only for the September iteration.
Can you do that?

@amanbha amanbha changed the title Merge Actor activation and first method call to user service. Merge Actor activation and first method/reminder call to user service. Aug 15, 2019
@amanbha
Copy link
Contributor Author

amanbha commented Aug 15, 2019

Corresponding spec issue: https://github.com/actionscore/spec/issues/10

@yaron2
Copy link
Member

yaron2 commented Aug 24, 2019

@amanbha I have some bandwidth to complete this for the current iteration, but that would involve a change in the SDK as well. Do you think you can make it or should we leave this item for the next iteration?

@amanbha
Copy link
Contributor Author

amanbha commented Aug 24, 2019

@yaron2 Lets do it in next iteration.

@amanbha
Copy link
Contributor Author

amanbha commented Aug 29, 2019

Actions runtime will not be able to distinguish between ActorActivation failure and method invocation failure. So it will try to activate the actor again even if the ActorActivation succeeded and only method invocation failed. Instead Actions runtime will revert the change to merge the calls made in this PR# #280

@amanbha amanbha reopened this Aug 29, 2019
@amanbha amanbha changed the title Merge Actor activation and first method/reminder call to user service. [Revert ]Merge Actor activation and first method/reminder call to user service. Aug 29, 2019
yaron2 added a commit that referenced this issue Aug 29, 2019
* revert #219

* try unset proxy

* remove unset proxy
@yaron2 yaron2 closed this as completed Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants