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

Updated Documentation for the Review endpoints #14401

Draft
wants to merge 17 commits into
base: dev
Choose a base branch
from

Conversation

iursevla
Copy link
Contributor

@iursevla iursevla commented Oct 17, 2024

Proposed change

This PR improves the documentation of the Review controller endpoints

Examples image image

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • The code has been formatted using Ruff (ruff format frigate)

Copy link

netlify bot commented Oct 17, 2024

Deploy Preview for frigate-docs canceled.

Name Link
🔨 Latest commit be68e36
🔍 Latest deploy log https://app.netlify.com/sites/frigate-docs/deploys/67110bb04069540008e51349

Copy link
Sponsor Collaborator

@NickM-27 NickM-27 left a comment

Choose a reason for hiding this comment

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

I think we should go ahead and create sub folders in defs for response and request to keep them more separate

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 already exists, we should not be recereating it

class SeverityEnum(str, Enum):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But that enum does not containsignificant_motion. Is it okay to add it to the enum you've linked?

class SeverityEnum(str, Enum):
    alert = "alert"
    detection = "detection"
    significant_motion = "significant_motion"

Copy link
Sponsor Collaborator

Choose a reason for hiding this comment

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

significant_motion does not exist currently so there is no reason for it to be added

@@ -558,38 +556,48 @@ def audio_activity(params: ReviewActivityMotionQueryParams = Depends()):

# change types for output
df.index = df.index.astype(int) // (10**9)
normalized = df.reset_index().to_dict("records")
normalized = json.loads(df.reset_index().to_json(orient="records"))
Copy link
Sponsor Collaborator

Choose a reason for hiding this comment

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

why is this being changed? it should stay the same just like motion

Copy link
Contributor Author

@iursevla iursevla Oct 17, 2024

Choose a reason for hiding this comment

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

I think it's due to the 'nan' values:

image

I left a FIXME comment above https://github.com/blakeblackshear/frigate/pull/14401/files#diff-8e868833994d25351027aea083642480ec208331a12238e7f009fd2e9b2e764fR550

Maybe we should fix it and then this does not happen anymore 🤔

Error:

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/uvicorn/protocols/http/h11_impl.py", line 406, in run_asgi
    result = await app(  # type: ignore[func-returns-value]
  File "/usr/local/lib/python3.9/dist-packages/uvicorn/middleware/proxy_headers.py", line 70, in __call__
    return await self.app(scope, receive, send)
  File "/usr/local/lib/python3.9/dist-packages/fastapi/applications.py", line 1054, in __call__
    await super().__call__(scope, receive, send)
  File "/usr/local/lib/python3.9/dist-packages/starlette/applications.py", line 113, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/errors.py", line 187, in __call__
    raise exc
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/errors.py", line 165, in __call__
    await self.app(scope, receive, _send)
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/base.py", line 189, in __call__
    response_sent.set()
  File "/usr/lib/python3.9/contextlib.py", line 135, in __exit__
    self.gen.throw(type, value, traceback)
  File "/usr/local/lib/python3.9/dist-packages/starlette/_utils.py", line 83, in collapse_excgroups
    raise exc
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/base.py", line 187, in __call__
    response = await self.dispatch_func(request, call_next)
  File "/usr/local/lib/python3.9/dist-packages/slowapi/middleware.py", line 136, in dispatch
    response = await call_next(request)
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/base.py", line 163, in call_next
    raise app_exc
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/base.py", line 149, in coro
    await self.app(scope, receive_or_disconnect, send_no_error)
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/base.py", line 189, in __call__
    response_sent.set()
  File "/usr/lib/python3.9/contextlib.py", line 135, in __exit__
    self.gen.throw(type, value, traceback)
  File "/usr/local/lib/python3.9/dist-packages/starlette/_utils.py", line 83, in collapse_excgroups
    raise exc
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/base.py", line 187, in __call__
    response = await self.dispatch_func(request, call_next)
  File "/workspace/frigate/frigate/api/fastapi_app.py", line 78, in frigate_middleware
    response = await call_next(request)
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/base.py", line 163, in call_next
    raise app_exc
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/base.py", line 149, in coro
    await self.app(scope, receive_or_disconnect, send_no_error)
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/base.py", line 189, in __call__
    response_sent.set()
  File "/usr/lib/python3.9/contextlib.py", line 135, in __exit__
    self.gen.throw(type, value, traceback)
  File "/usr/local/lib/python3.9/dist-packages/starlette/_utils.py", line 83, in collapse_excgroups
    raise exc
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/base.py", line 187, in __call__
    response = await self.dispatch_func(request, call_next)
  File "/usr/local/lib/python3.9/dist-packages/starlette_context/middleware/context_middleware.py", line 78, in dispatch
    response = await call_next(request)
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/base.py", line 163, in call_next
    raise app_exc
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/base.py", line 149, in coro
    await self.app(scope, receive_or_disconnect, send_no_error)
  File "/usr/local/lib/python3.9/dist-packages/starlette/middleware/exceptions.py", line 62, in __call__
    await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send)
  File "/usr/local/lib/python3.9/dist-packages/starlette/_exception_handler.py", line 62, in wrapped_app
    raise exc
  File "/usr/local/lib/python3.9/dist-packages/starlette/_exception_handler.py", line 51, in wrapped_app
    await app(scope, receive, sender)
  File "/usr/local/lib/python3.9/dist-packages/starlette/routing.py", line 715, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/usr/local/lib/python3.9/dist-packages/starlette/routing.py", line 735, in app
    await route.handle(scope, receive, send)
  File "/usr/local/lib/python3.9/dist-packages/starlette/routing.py", line 288, in handle
    await self.app(scope, receive, send)
  File "/usr/local/lib/python3.9/dist-packages/starlette/routing.py", line 76, in app
    await wrap_app_handling_exceptions(app, request)(scope, receive, send)
  File "/usr/local/lib/python3.9/dist-packages/starlette/_exception_handler.py", line 62, in wrapped_app
    raise exc
  File "/usr/local/lib/python3.9/dist-packages/starlette/_exception_handler.py", line 51, in wrapped_app
    await app(scope, receive, sender)
  File "/usr/local/lib/python3.9/dist-packages/starlette/routing.py", line 73, in app
    response = await f(request)
  File "/usr/local/lib/python3.9/dist-packages/fastapi/routing.py", line 301, in app
    raw_response = await run_endpoint_function(
  File "/usr/local/lib/python3.9/dist-packages/fastapi/routing.py", line 214, in run_endpoint_function
    return await run_in_threadpool(dependant.call, **values)
  File "/usr/local/lib/python3.9/dist-packages/starlette/concurrency.py", line 39, in run_in_threadpool
    return await anyio.to_thread.run_sync(func, *args)
  File "/usr/local/lib/python3.9/dist-packages/anyio/to_thread.py", line 56, in run_sync
    return await get_async_backend().run_sync_in_worker_thread(
  File "/usr/local/lib/python3.9/dist-packages/anyio/_backends/_asyncio.py", line 2405, in run_sync_in_worker_thread
    return await future
  File "/usr/local/lib/python3.9/dist-packages/anyio/_backends/_asyncio.py", line 914, in run
    result = context.run(func, *args)
  File "/workspace/frigate/frigate/api/review.py", line 560, in audio_activity
    return JSONResponse(content=normalized)
  File "/usr/local/lib/python3.9/dist-packages/starlette/responses.py", line 178, in __init__
    super().__init__(content, status_code, headers, media_type, background)
  File "/usr/local/lib/python3.9/dist-packages/starlette/responses.py", line 41, in __init__
    self.body = self.render(content)
  File "/usr/local/lib/python3.9/dist-packages/starlette/responses.py", line 181, in render
    return json.dumps(
  File "/usr/lib/python3.9/json/__init__.py", line 234, in dumps
    return cls(
  File "/usr/lib/python3.9/json/encoder.py", line 199, in encode
    chunks = self.iterencode(o, _one_shot=True)
  File "/usr/lib/python3.9/json/encoder.py", line 257, in iterencode
    return _iterencode(o, 0)
ValueError: Out of range float values are not JSON compliant

Copy link
Sponsor Collaborator

Choose a reason for hiding this comment

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

We are not currently using this in the UI, I think the part commented on should stay the same, perhaps it can be adjusted to work the same as the motion or otherwise we can just delete this API since it is currently not used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think we can remove it then? @blakeblackshear @hawkeye217 is this okay for you guys too? I prefer to have only code that is used.

@NickM-27
Copy link
Sponsor Collaborator

NickM-27 commented Oct 17, 2024

sorry, didn't realize it was still in draft, the email link took me right to the file changes

@iursevla
Copy link
Contributor Author

sorry, didn't realize it was still in draft, the email link took me right to the file changes

No worries. Those were good comments

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