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

Polling topic model #474

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

Polling topic model #474

wants to merge 28 commits into from

Conversation

markbotterill
Copy link
Contributor

@markbotterill markbotterill commented Jan 10, 2025

Reviewer: @sidravi1 / @tonyzhao6 / @lickem22
Estimate: 45 mins


Ticket

Fixes: JIRA_TICKET_LINK

Description

Previously the Topic Modeling request took ~20-40 seconds. As per Amir's PR we now get nicer loading states. However, this is not best practice for FastAPI / production systems. We shouldn't need to increase time-outs (default @ 5s) to get around this.

Instead, we now kick of a background job and FastAPI instantly returns a "Job started" response. The progress of that job is then continuously written to the Redis (including fail states). If for any reason, the run fails, the user is given a notification and a reason in the frontend. Logs are also written to confirm in the backend.

I've also built on Amir's UI updates to track run states while being able to navigate between different time periods.

Goal

Reduce issues with deployment + make the UI experience smoother.

Changes

  1. Integrated Background call
  2. Add "steps" to the TopicData response + changed them throughout router.py. This allows us to easily track where failures occur if a step doesn't complete.
  3. Add polling (default ever 3s to reduce API burden) to the frontend so we can check progress by pinging Redis.
  4. Add states to track the loading states / polling states of refresh runs.
  5. Better error logging added to topic modeling run + ensure minimum 500 queries + contents in system before executing run.

Future Tasks (optional)

Toast pop-ups can glitch slightly sometimes. (Lower priority)
Documentation probably needs updating -> full technical docs page for topic modeling needed.

How has this been tested?

Both with docker compose + local testing. Tests still run fine.

Steps to Test

  1. Spin up in dev mode.
  2. Make sure > 500 queries are in the system + at least one content card (to create queries quickly, run add_dummy_data_to_db.py in backend)
  3. Run topic modeling across various time steps from the frontend.

To-do before merge (optional)

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

(Delete any items below that are not relevant)

  • I have updated the automated tests
  • I have updated the scripts in scripts/
  • I have updated the requirements
  • I have updated the README file
  • I have updated affected documentation
  • I have added a blogpost in Latest Updates
  • I have updated the CI/CD scripts in .github/workflows/
  • I have updated the Terraform code

amiraliemami and others added 27 commits January 9, 2025 17:10
@markbotterill markbotterill marked this pull request as ready for review January 13, 2025 09:59
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