Skip to content
This repository was archived by the owner on Aug 20, 2025. It is now read-only.

Fastapi#68

Merged
DanielElisenberg merged 13 commits intomainfrom
fastapi
Jul 1, 2025
Merged

Fastapi#68
DanielElisenberg merged 13 commits intomainfrom
fastapi

Conversation

@DanielElisenberg
Copy link
Contributor

Rewrite metadata-service to FastAPI

Rewrote without touching much else than the api layer and the absolute necessary stuff that was affected by the switch.

Notes

  • We used to compress in middleware. This is tricky to do in the same way because of how the asynchronous responses are built. Solved it by making a custom response model that compresses in the same way, using the same dependency, and a factory function for Depends to avoid boilerplate. There is a test that tests this by unpacking the content with msgpack and it still runs 👍🏻 ✅
  • lru_cache can be kept with no issue for now as it is thread-safe within a process and the function is not async

Question

In order to make the app even more efficient I first thought of making the file reading asynchronous. This makes the lru_cache not work anymore and our options would be:

  • use the async-lru crate which is microscopic and questionably maintained. Probably doesnt need to be very often maintained tho as it is so small
  • write a simple async cache in-app. This shouldn't be harder than a small class and an async.lock

But after weighing these options, where 2 was the clear winner, I decided to not do anything at all. metadata_alls are immutable so with the cache we read them RARELY. AKA no real perfomance boosts anyway.

If we want we can make reading the draft asynchronous in the future, as it is not cached. We are not seeing any complaints about draft performance yet though, so I opted to just wait with this.

Does this sound OK?

@DanielElisenberg DanielElisenberg requested a review from a team as a code owner June 26, 2025 13:24
Copy link
Collaborator

@pawbu pawbu left a comment

Choose a reason for hiding this comment

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

Yes, let's wait with further optimizations until they're needed 👍

@sonarqubecloud
Copy link

Copy link
Contributor

@linnbjornerud linnbjornerud left a comment

Choose a reason for hiding this comment

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

Things are looking good 💯 Time for testing I suppose!

@DanielElisenberg DanielElisenberg merged commit 94babbc into main Jul 1, 2025
7 checks passed
@DanielElisenberg DanielElisenberg deleted the fastapi branch July 1, 2025 08:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants