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.

Sign up for GitHub

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

Jump to bottom

Persist topic modelling loading state #451

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Persist topic modelling loading state #451

wants to merge 5 commits into from

Conversation

amiraliemami
Copy link
Collaborator

Reviewer: @lickem22
Estimate: 30mins

Ticket

Fixes: 837

Description

Goal

  • Topic modelling button's loading state should persist even after page navigations

Changes

  • Added a "status" key to the TopicData type and now using it to track the status of topic modelling jobs.

How has this been tested?

  • Run local dev
  • Add dummy data
  • click on generate topics and navigate and come back

Checklist

Fill with x for completed.

  • My code follows the style guidelines of this project
  • I have reviewed my own code to ensure good quality
  • I have tested the functionality of my code to ensure it works as intended
  • I have resolved merge conflicts

lickem22
lickem22 reviewed Oct 22, 2024
View reviewed changes
core_backend/app/dashboard/schemas.py
@@ -196,6 +196,7 @@ class TopicsData(BaseModel):
of individual Topics. Used for Insights page.
"""

status: Literal["not_started", "in_progress", "completed", "error"]
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about using numbers instead of string to avoid errors?

@amiraliemami
Copy link
Collaborator Author

To fix:

  • When running it says "Last run: Never". Make sure to pass last run through to the in_progress payload
  • When the button is clicked, clicking on other tabs still shows loading. Fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lickem22 lickem22 lickem22 left review comments

@sidravi1 sidravi1 Awaiting requested review from sidravi1 sidravi1 is a code owner

@suzinyou suzinyou Awaiting requested review from suzinyou suzinyou is a code owner

@Tanmay-97 Tanmay-97 Awaiting requested review from Tanmay-97 Tanmay-97 is a code owner

@markbotterill markbotterill Awaiting requested review from markbotterill markbotterill is a code owner

@tonyzhao6 tonyzhao6 Awaiting requested review from tonyzhao6 tonyzhao6 is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@amiraliemami @lickem22