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

[BUG] Fix ArrayNode not reporting terminal state in TaskExecutionEvent #5135

Open
2 tasks done
pvditt opened this issue Mar 29, 2024 · 4 comments
Open
2 tasks done

[BUG] Fix ArrayNode not reporting terminal state in TaskExecutionEvent #5135

pvditt opened this issue Mar 29, 2024 · 4 comments
Assignees
Labels
bug Something isn't working exo

Comments

@pvditt
Copy link
Contributor

pvditt commented Mar 29, 2024

Describe the bug

ArrayNode currently reports events identically to maptask plugin where we encapsulate subNode executions within the ExternalResourceInfo field in TaskExecutionEvent proto message. The ExternalResourceInfo proto message has a phase field, which corresponds to the phase of the subNode. The top-level node phase is reported using (and displayed in the UI from) the NodeExecutionEvent. So there is a task phase on the TaskExecutionEvent that is not currently being used.

However, this phase is incorrect. We attempt to set it on terminal states here , but this code is never reached because terminal states return prematurely. We need to ensure that the phase on the TaskExecutionEvent is correctly set on terminal phases.

Expected behavior

task execution phases match terminal state for array nodes

Additional context to reproduce

No response

Screenshots

No response

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@pvditt pvditt added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers exo labels Mar 29, 2024
@pvditt pvditt self-assigned this Mar 29, 2024
@eapolinario eapolinario removed the untriaged This issues has not yet been looked at by the Maintainers label Apr 4, 2024
@cw75
Copy link

cw75 commented Apr 22, 2024

@RunLLM is there a pull request that fixes this issue?

Copy link

runllm bot commented Apr 22, 2024

To address the issue of ArrayNode not reporting the terminal state in TaskExecutionEvent, a pull request titled "[Bug] correctly set task execution phase for terminal array node" (PR #5136) is currently open. This PR aims to ensure the correct phase is set in task execution events for terminal ArrayNodes. It is still under review and not yet merged.

Would you like more details on the changes proposed in this pull request?

Sources:


This is an AI-generated response and your feedback is appreciated! Please leave a 👍 if this is helpful and 👎 if it is not.

Mention @RunLLM to ask follow-up questions. See runllm.com for more!

@pvditt
Copy link
Contributor Author

pvditt commented Aug 22, 2024

fixes:
#5680
#5681
#5451

oops - this is the incorrect issue for these

@pvditt
Copy link
Contributor Author

pvditt commented Aug 22, 2024

@cw75 we just upstreamed a few of the fixes. Will close this after #5681 is merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working exo
Projects
None yet
Development

No branches or pull requests

3 participants