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.

Sign up for GitHub

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

backend support for upload modal #89

Merged
merged 3 commits into from
Nov 30, 2024
Merged

backend support for upload modal #89

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
554fd80
backend support for upload modal
0xrohitgarg Nov 28, 2024
7ea60cf
add secure filename validation and error handling
0xrohitgarg Nov 29, 2024
5e2c65c
remove file validation
0xrohitgarg Nov 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions backend/director/entrypoint/api/routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import os

from flask import Blueprint, request, current_app as app
from werkzeug.utils import secure_filename

from director.db import load_db
from director.handler import ChatHandler, SessionHandler, VideoDBHandler, ConfigHandler
Expand All @@ -11,6 +12,8 @@
videodb_bp = Blueprint("videodb", __name__, url_prefix="/videodb")
config_bp = Blueprint("config", __name__, url_prefix="/config")

SUPPORTED_MEDIA_TYPES = ["audio/mpeg", "video/mp4", "image/jpeg", "image/png"]

Copy link
Contributor

@coderabbitai coderabbitai bot Nov 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Implement media type validation using the SUPPORTED_MEDIA_TYPES constant.

The SUPPORTED_MEDIA_TYPES constant is defined but not utilized. This should be used to implement the TODO comment on line 104 for media type validation.

Here's how to implement the validation:

            media_type = file.content_type.split("/")[0]
+           if file.content_type not in SUPPORTED_MEDIA_TYPES:
+               return {"message": f"Unsupported media type. Supported types are: {', '.join(SUPPORTED_MEDIA_TYPES)}"}, 400
-           # TODO: Check if the media type is supported, if not, return 400
            return videodb.upload(

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

let the internal function of conn.upload handle the validation, if it not a valid type, the function should throw error, we are going to catch the error and simply pass that to client.

Copy link
Contributor

Choose a reason for hiding this comment

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

@0xrohitgarg, understood. If the conn.upload function handles the media type validation and raises an error for unsupported types, and you're catching the exception and passing it to the client, that approach works well.

✏️ Learnings added 
Learnt from: 0xrohitgarg
PR: video-db/Director#89
File: backend/director/entrypoint/api/routes.py:0-0
Timestamp: 2024-11-29T05:47:14.334Z
Learning: In `backend/director/entrypoint/api/routes.py`, the `upload_video` function relies on `conn.upload` for media type validation. The `conn.upload` function in `backend/director/tools/videodb_tool.py` raises an error if the media type is unsupported, and exceptions are caught and passed to the client in `upload_video`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


@agent_bp.route("/", methods=["GET"], strict_slashes=False)
def agent():
Expand Down Expand Up @@ -85,6 +88,37 @@ def get_video_or_all(collection_id, video_id):
return videodb.get_videos()


@videodb_bp.route("/collection/<collection_id>/upload", methods=["POST"])
def upload_video(collection_id):
"""Upload a video to a collection."""
try:
videodb = VideoDBHandler(collection_id)

if "file" in request.files:
file = request.files["file"]
file_bytes = file.read()
safe_filename = secure_filename(file.filename)
if not safe_filename:
return {"message": "Invalid filename"}, 400
file_name = os.path.splitext(safe_filename)[0]
# TODO: Check if the media type is supported, if not, return 400
media_type = file.content_type.split("/")[0]
return videodb.upload(
source=file_bytes,
source_type="file",
media_type=media_type,
name=file_name,
)
elif "source" in request.json:
source = request.json["source"]
source_type = request.json["source_type"]
return videodb.upload(source=source, source_type=source_type)
else:
return {"message": "No valid source provided"}, 400
except Exception as e:
return {"message": str(e)}, 500


@config_bp.route("/check", methods=["GET"])
def config_check():
config_handler = ConfigHandler()
Expand Down
5 changes: 5 additions & 0 deletions backend/director/handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ class VideoDBHandler:
def __init__(self, collection_id):
self.videodb_tool = VideoDBTool(collection_id=collection_id)

def upload(
self, source, source_type="url", media_type="video", name=None
):
return self.videodb_tool.upload(source, source_type, media_type, name)

def get_collection(self):
"""Get a collection by ID."""
return self.videodb_tool.get_collection()
Expand Down
12 changes: 11 additions & 1 deletion backend/director/tools/videodb_tool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import os
import requests
import videodb

from videodb import SearchType, SubtitleStyle, IndexType, SceneExtractionType
from videodb.timeline import Timeline
from videodb.asset import VideoAsset, ImageAsset



class VideoDBTool:
def __init__(self, collection_id="default"):
self.conn = videodb.connect(
Expand Down Expand Up @@ -81,6 +81,16 @@ def upload(self, source, source_type="url", media_type="video", name=None):
upload_args["name"] = name
if source_type == "url":
upload_args["url"] = source
elif source_type == "file":
upload_url_data = self.conn.get(
path=f"/collection/{self.collection.id}/upload_url",
params={"name": name},
)
upload_url = upload_url_data.get("upload_url")
files = {"file": (name, source)}
response = requests.post(upload_url, files=files)
response.raise_for_status()
upload_args["url"] = upload_url
else:
upload_args["file_path"] = source
media = self.conn.upload(**upload_args)
Expand Down