Switching background tasks for threads on client and server updates

[lotif]

PR Type

Fix

Short Description

Clickup Ticket(s): NA

While I was testing the app this week, I realized FastAPI's Background Tasks, which we use for client and server metric updates, are not really suitable for long running tasks as they are executed serially instead of in parallel.

Here I am switching from that to plain python threads, and also making a few other changes:

• Adding a database connection in the listeners as FastAPI's db instance cannot be shared between threads
• Changing the listener functions to be async
• Converting the sync database functions to async in the Job
• Getting rid of the sync database connection, we don't need it anymore
• For metrics reporter, avoid saving the metrics if it's exactly the same as what's already stored. This will eliminate an issue with updates that are too frequent into Redis with no new information, which makes the app do a lot of unnecessary work.
• Fixing the metrics in the UI progress section for the updated metric names

Tests Added

Fixed unit and integration tests accordingly

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 92.15686% with 8 lines in your changes missing coverage. Please review.

Project coverage is 93.99%. Comparing base (a6d0cf3) to head (729ba67).

Files with missing lines	Patch %	Lines
florist/app/jobs/details/page.tsx	86.04%	6 Missing ⚠️
florist/api/routes/server/training.py	95.55%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #122      +/-   ##
==========================================
+ Coverage   89.85%   93.99%   +4.14%     
==========================================
  Files          23       24       +1     
  Lines        2129     2166      +37     
  Branches      179      120      -59     
==========================================
+ Hits         1913     2036     +123     
+ Misses        216      130      -86     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[emersodb]

This is sort of an interesting structure that I'm not super familiar with. For my own curiosity, what is happening when you do db_client[DATABASE_NAME]? I've only ever seen [...] for generics in mypy

[emersodb]

Ah...this was probably a dumb question. I assume this is something like a map that retrieves the database from the key 🤦

[lotif]

This is an interesting thing, I went a bit deep into that because I had to figure out how to mock it for unit tests haha. It is not a map/dictionary, it's actually a class that implements the __getitem__ function, which makes it also behave like a dictionary. I guess they thought it's a bit cleaner to do that than to implement a get_database kind of method but it makes it a bit confusing at first. At least I learned something :)

See:

FLorist/florist/tests/unit/api/routes/server/test_training.py

Line 625 in 3da12ea

mock_db_client.__getitem__ = Mock(

[emersodb]

Gotcha...very similar, but much less obvious what's happening 😂

[emersodb]

These changes look good to me. I'm definitely not an expert in database communication, but the threading change seems fairly straightforward.