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

POC: Frigate HTTP API FastAPI #13324

Closed
wants to merge 30 commits into from
Closed

Conversation

iursevla
Copy link

@iursevla iursevla commented Aug 24, 2024

FastAPI POC

This is a POC of using FastAPI to replace Flask in Frigate HTTP API. It is using the defaults provided by FastAPI (uvicorn and starlette). This was discussed here. The intent is not to be merged right away but rather to further the discussion and to get your feedback on my proposals at the end of this description.

I'm proposing that we start moving to FastAPI by mounting Flask as a sub-application sub-application + 2. This means that the entire application still works as is and we can then start moving endpoint by endpoint to FastAPI until we get to a point where all endpoints are converted. At that point, we can remove Flask altogether.

I've started the conversion of 2-3 endpoints to showcase how it works:

  • Preview endpoints:
    • /preview/{camera_name}/start/{start_ts}/end/{end_ts}
    • /preview/{camera_name}/start/{start_ts}/end/{end_ts}/frames
  • Media endpoints:
    • /{camera_name}/latest.{extension} -> See warning below
  • App endpoints:
    • /logs/{service}

Here it is working with FastAPI

converted_file.mp4

HTTP API Documentation

Without any extra effort, the swagger documentation is available at http://localhost:5001/docs or http://localhost:5001/redoc See documentation

See image && video image
converted_file.mp4
Screenshot 2024-08-24 at 16 05 59

The JSON/YAML schema can then be used to feed the HTTP API page without any human interaction.

From this documentation we can quickly create a client to be used by the front end. This can be done automatically with openapi-generator-cli. I've used this successfully multiple times at work and also for personal projects, e.g., Frigate HTTP API + 2.


Problems faced

Caution

I've faced a problem with the existing endpoints which have an extension suffix: .(jpg|jpeg|png|webp|gif).As a result ⚠️ This lines in nginx.conf was commented out. I've left a comment explaining the errors. If someone wants/can help me figure out why it is failing, then use my branch to debug the endpoints that contain an extension.


Proposal(s):

My main proposal is (assuming we can get the nginx problem I've faced resolved):

Other proposals:

  1. In the previous POC I was not specific about the types for each path/query parameter but we can be more precise with the types
Details image

By being specific the endpoint will fail if invoked with invalid parameters

image

and we also get extra auto-complete on the docs

image
  1. Add prefix to API endpoints e.g. media related endpoints should be /api/media/... this avoid any conflict between endpoints and makes it easier to read and more "RESTFUL".

    i.e., the following should all be `/api/media/...` image

This can also be impactful if the user names the camera events which would end up requesting a different endpoint than expect (i.e., conflicts between ("/events/<id>/thumbnail.jpg" and "/<camera_name>/<label>/thumbnail.jpg"))

  1. Split app.py file into multiple other route files:
    • Move the config endpoints to config.py
    • Move the timeline endpoints to timeline.py
    • Move the logs endpoints tologs.py
    • Move the app lifecycle endpoints (stats/version/restart/etc) to lifecycle.py

Proposals 2 and 3 probably should be left out for future implementation to avoid scope creep

hunterjm and others added 30 commits August 23, 2024 07:00
* Initial re-implementation of semantic search

* put docker-compose back and make reindex match docs

* remove debug code and fix import

* fix docs

* manually build pysqlite3 as binaries are only available for x86-64

* update comment in build_pysqlite3.sh

* only embed objects

* better error handling when genai fails

* ask ollama to pull requested model at startup

* update ollama docs

* address some PR review comments

* fix lint

* use IPC to write description, update docs for reindex

* remove gemini-pro-vision from docs as it will be unavailable soon

* fix OpenAI doc available models

* fix api error in gemini and metadata for embeddings
* initial event search api implementation

* fix lint

* fix tests

* move chromadb imports and pysqlite hotswap to fix tests

* remove unused import

* switch default limit to 50

* fix events accidently pulling inside chroma results loop
* Add basic search page

* Abstract filters to separate components

* Make searching functional

* Add loading and no results indicators

* Implement searching

* Combine account and settings menus on mobile

* Support using thumbnail for in progress detections

* Fetch previews

* Move recordings view and open recordings when search is selected

* Implement detail pane

* Implement saving of description

* Implement similarity search

* Fix clicking

* Add date range picker

* Fix

* Fix iOS zoom bug

* Mobile fixes

* Use text area

* Fix spacing for drawer

* Fix fetching previews incorrectly
* Chroma logs in frontend

* fix lint
* Initial support for Hailo-8L

Added file for Hailo-8L detector including dockerfile, h8l.mk, h8l.hcl, hailo8l.py, ci.yml and ssd_mobilenat_v1.hef as the inference network.

Added files to help with the installation of Hailo-8L dependences like generate_wheel_conf.py, requirements-wheel-h8l.txt and modified setup.py to try and work with any hardware.

Updated docs to reflect Initial Hailo-8L support including oject_detectors.md,  hardware.md and installation.md.

* Update .github/workflows/ci.yml

typo h8l not arm64

Co-authored-by: Nicolas Mowen <nickmowen213@gmail.com>

* Update docs/docs/configuration/object_detectors.md

Clarity for the end user and correct uses of words

Co-authored-by: Nicolas Mowen <nickmowen213@gmail.com>

* Update docs/docs/frigate/installation.md

typo

Co-authored-by: Nicolas Mowen <nickmowen213@gmail.com>

* update Installation.md to clarify Hailo-8L installation process.

* Update docs/docs/frigate/hardware.md

Co-authored-by: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com>

* Update hardware.md add Inference time.

* Oops no new line at the end of the file.

* Update docs/docs/frigate/hardware.md typo

Co-authored-by: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com>

* Update dockerfile to download the ssd_modilenet_v1 model instead of having it in the repo.

* Updated dockerfile so it dose not download the model file.

add function to download it at runtime.

update model path.

* fix formatting according to ruff and removed unnecessary functions.

---------

Co-authored-by: Nicolas Mowen <nickmowen213@gmail.com>
Co-authored-by: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com>
* Setup basic notification page

* Add basic notification implementation

* Register for push notifications

* Implement dispatching

* Add fields

* Handle image and link

* Add notification config

* Add field for users notification tokens

* Implement saving of notification tokens

* Implement VAPID key generation

* Implement public key encoding

* Implement webpush from server

* Implement push notification handling

* Make notifications config only

* Add maskable icon

* Use zod form to control notification settings in the UI

* Use js

* Always open notification

* Support multiple endpoints

* Handle cleaning up expired notification registrations

* Correctly unsubscribe notifications

* Change ttl dynamically

* Add note about notification latency and features

* Cleanup docs

* Fix firefox pushes

* Add links to docs and improve formatting

* Improve wording

* Fix docstring

Co-authored-by: Blake Blackshear <blake@frigate.video>

* Handle case where native auth is not enabled

* Show errors in UI

---------

Co-authored-by: Blake Blackshear <blake@frigate.video>
* Disable semantic search by default and don't start processes unless enabled

* Conditionally create embeddings

* Fix typing
* Support building docker image for amd64 and arm64

* Support installations on amd64 and arm64

* Run build in new job

* Update docs
* Use review item thumbnail for export

* Formatting
…hear#13071)

* Add ability to submit to frigate+ from review panel

* Add separator

* Use consistent ID
@iursevla iursevla marked this pull request as ready for review August 24, 2024 17:09
proxy_pass http://frigate_api;
include proxy.conf;
}
# FIXME: Needed to disabled this rule, otherwise it fails for endpoints that end with one of those file extensions
Copy link
Sponsor Collaborator

Choose a reason for hiding this comment

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

I'd like to understand better what exactly was wrong with this endpoint

Copy link
Author

Choose a reason for hiding this comment

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

Yes, me too. I tried to do some debugging and always ended up on the same line of code: https://github.com/encode/uvicorn/blob/47304d9ae76321f0f5f649ff4f73e09b17085933/uvicorn/protocols/http/httptools_impl.py#L165

If I invoked the API directly (i.e., localhost:5001 instead of localhost:5173/api...) it works just fine.

If any of you can take a look I'd appreciate.

To have more verbose logs and debug the code further I did the following:

Details image image image

]


def create_fastapi_app(frigate_config, detected_frames_processor):
Copy link
Sponsor Collaborator

Choose a reason for hiding this comment

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

this is good to see, I was not sure how hard it would be to map these additional objects into the api class so they would be available to the handlers

@iursevla
Copy link
Author

iursevla commented Sep 1, 2024

@NickM-27 @blakeblackshear how do you guys think I should go with this?

I'm continuing the conversion (You can see my progress here: poc-fastapi-continued), using the dev branch as the base (and I'll keep it up to date with upstream).

I'll convert all endpoints to FastAPI to see how it looks in the end.

@blakeblackshear
Copy link
Owner

I don't want to discourage you, but we are just focused on other things at the moment. I think the best path forward is to open a new PR. This one seems to have gotten a bit tangled up with other changes, so it was hard to review. In general, I like the approach you outlined where we lay the ground work and incrementally port over the endpoints.

I can probably track down those problematic endpoints once you open a new PR.

@iursevla iursevla mentioned this pull request Sep 7, 2024
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.

6 participants