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

Re-execute Node even when there is a persisted response #45

Closed
kijujjav opened this issue Aug 25, 2021 · 2 comments
Closed

Re-execute Node even when there is a persisted response #45

kijujjav opened this issue Aug 25, 2021 · 2 comments

Comments

@kijujjav
Copy link
Collaborator

kijujjav commented Aug 25, 2021

Hi All,

We have an use case where we execute a node and if none of the child selectors match, we want to retry the same node after few min to see if any of the child selectors match. But the current behavior is in a way that if a persistent response is found the execution of the node is skipped.

How can I have the node to be re-executed even when the persisted response is found?

This is also helpful when we have loops in the forge schema

@TravisOnGit
Copy link
Member

Hey there, great question!

For clarification, you have an ActionType node where the ChildSelectors do not find a match and return. When you retry by calling WalkTree on the same CurrentTreeNode some minutes later, you want the Actions to execute again. If that's not what you are asking, please let me know.

You have a few options. Let me give you some context/information first. Forge supports cycles (i.e. revisiting TreeNodes). You can find the logic and comments here: https://github.com/microsoft/Forge/blob/master/Forge.TreeWalker/src/TreeWalkerSession.cs#L1117

// If we are revisiting a node that has been previously completed and we aren't rehydrating,
// clear the ActionResponses and Intermediates, and persist PreviousActionResponses.

Forge is considered "Rehydrating" in this context until it has successfully committed the CurrentTreeNode. So in the case where you create a new TreeWalkerSession and retry WalkTree at your desired TreeNode, the hasSessionRehydrated flag is false, and therefore the cycle logic does not get executed. This is the expected behavior, to NOT retry by default. If we are rehydrating and the ActionResponses already exist on the TreeNode, that means the Actions have already finished executing so we skip to the ChildSelection. We intentionally do not re-execute the Actions.

With that context, here are some options:

  1. Satisfy the hasSessionRehydrated flag by visiting a TreeNode that points to your desired retry_TreeNode.
    You don't have to retry WalkTree on the CurrentTreeNode you left off on. You can call WalkTree from any TreeNode, anytime. If your retry_TreeNode had a simple parent TreeNode with a single ChildSelector, you could WalkTree on this parent TreeNode. This would allow hasSessionRehydrated to become true, and your retry_TreeNode would get it's ActionResponses wiped and Actions re-executed.
  2. Instead of not matching a ChildSelector, go to a child with an Action that runs Task.Delay for the desired amount of minutes with a ChildSelector pointing back to your desired retry_TreeNode.
    This takes advantage of Forge's built-in cycle logic by revisiting the TreeNode in the same WalkTree session.
  3. Manually remove the ActionResponses that you wish to retry from the IForgeDictionary between retries.
    When you then call WalkTree on your desired retry_TreeNode, the ActionResponses will not exist so the Actions will get re-executed.

Overall I would lean towards option 2, since the other options are a bit hacky. But anything should work.

Also, if you agree, we could create a feature request to add a "RetryCurrentTreeNodeActions" property to TreeWalkerParameters. This would basically override the hasSessionRehydrated check and always re-execute Actions whenever a TreeNode is visited. Let me know if this would be useful to you, and we can look at putting it in the next release.

Thanks!
--Travis

@TravisOnGit
Copy link
Member

Thank you kijujjav for sending a pull request to add a new RetryCurrentTreeNodeActions flag. The latest released version (1.0.59) contains the flag. Closing this issue on this PR: #46

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

No branches or pull requests

2 participants