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

Endpoint Interruption Logic for Model Overview with OD #2268

Merged
merged 3 commits into from
Aug 24, 2023

Conversation

Advitya17
Copy link
Collaborator

@Advitya17 Advitya17 commented Aug 22, 2023

mo_endpoint_1.mp4

Description

For scenarios such as OD & QA that use an endpoint to calculate Model Overview metrics, even if the metrics are drawn from the cache, there's still an execution invoked for the endpoint to correctly trigger the state change.

If there're any user interaction related to changing cohorts or quick user input changes while a previous endpoint instance is running, the dashboard would fall into an error. This PR adds abort logic to the endpoint such that a change in user inputs aborts the previous execution to avoid errors in the state or cohort values.

A future PR would add corresponding support to QA as well.

Checklist

  • I have added screenshots above for all UI changes.
  • I have added e2e tests for all UI changes.
  • Documentation was updated if it was needed.

Copy link
Contributor

@romanlutz romanlutz left a comment

Choose a reason for hiding this comment

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

Nice, also thank you for adding the video!

Have we considered adding a spinner while it's loading?

@Advitya17
Copy link
Collaborator Author

Nice, also thank you for adding the video!

Have we considered adding a spinner while it's loading?

Thanks for the review, @romanlutz! It has come up in discussions before, but we haven't put that in the backlog just yet. Currently we're making do with N/A until values are rendered, but could see in a future PR if spinners inside the table values is (a) feasible to put in (instead of a string), and (b) preferred PM decision.

Let me know your thoughts and can bring it to discussion accordingly.

1 similar comment
@Advitya17 Advitya17 merged commit 798e9d5 into main Aug 24, 2023
@Advitya17 Advitya17 deleted the agemawat/endpoint_abort branch August 24, 2023 16:06
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