-
Notifications
You must be signed in to change notification settings - Fork 69
[Feat] new video generation providers #368
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
Conversation
Caution Review failedThe pull request is closed. WalkthroughThis update introduces asynchronous video generation capabilities for both Google and Microsoft providers. New methods for launching and retrieving video generation jobs are added to their respective API classes, with integration for S3 storage. Corresponding API specifications and provider settings are updated to reflect these new features. Additionally, the Minimax API's job result retrieval method is enhanced to download and encode video content. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GoogleVideoApi
participant GoogleAPI
participant S3
Client->>GoogleVideoApi: video__generation_async__launch_job(params)
GoogleVideoApi->>GoogleAPI: POST /generate_video
GoogleAPI-->>GoogleVideoApi: provider_job_id
GoogleVideoApi-->>Client: AsyncLaunchJobResponseType
Client->>GoogleVideoApi: video__generation_async__get_job_result(job_id)
GoogleVideoApi->>GoogleAPI: GET /job_status
alt Job Complete
GoogleVideoApi->>GoogleAPI: Download video
GoogleVideoApi->>S3: Upload video
S3-->>GoogleVideoApi: S3 URL
GoogleVideoApi-->>Client: GenerationAsyncDataClass (video, S3 URL)
else Job Pending
GoogleVideoApi-->>Client: Pending response
end
sequenceDiagram
participant Client
participant MicrosoftVideoApi
participant AzureAPI
participant S3
Client->>MicrosoftVideoApi: video__generation_async__launch_job(params)
MicrosoftVideoApi->>AzureAPI: POST /video/generation/jobs
AzureAPI-->>MicrosoftVideoApi: provider_job_id
MicrosoftVideoApi-->>Client: AsyncLaunchJobResponseType
Client->>MicrosoftVideoApi: video__generation_async__get_job_result(job_id)
MicrosoftVideoApi->>AzureAPI: GET /video/generation/jobs/{job_id}
alt Job Succeeded
MicrosoftVideoApi->>AzureAPI: Download video
MicrosoftVideoApi->>S3: Upload video
S3-->>MicrosoftVideoApi: S3 URL
MicrosoftVideoApi-->>Client: GenerationAsyncDataClass (video, S3 URL)
else Job Pending
MicrosoftVideoApi-->>Client: Pending response
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (2)
edenai_apis/apis/google/google_video_api.py (1)
959-959
: Address the commented personGeneration parameter.The commented line suggests this feature might be needed. Please either implement it properly or remove the comment if it's not required.
Do you want me to help determine if this parameter is needed based on the Google API documentation?
edenai_apis/apis/microsoft/microsoft_video_api.py (1)
59-59
: Consider using a more standard HTTP status code check.The current check
> 201
is unusual. Consider using>= 400
ornot in [200, 201]
for clarity.-if response.status_code > 201: +if response.status_code >= 400:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
edenai_apis/apis/google/google_video_api.py
(4 hunks)edenai_apis/apis/google/info.json
(1 hunks)edenai_apis/apis/microsoft/info.json
(1 hunks)edenai_apis/apis/microsoft/microsoft_api.py
(2 hunks)edenai_apis/apis/microsoft/microsoft_video_api.py
(1 hunks)edenai_apis/features/video/generation_async/generation_async_args.py
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
edenai_apis/apis/microsoft/microsoft_api.py (1)
edenai_apis/apis/microsoft/microsoft_video_api.py (1)
MicrosoftVideoApi
(26-107)
🪛 Pylint (3.3.7)
edenai_apis/apis/microsoft/microsoft_video_api.py
[refactor] 27-27: Too many arguments (9/7)
(R0913)
[refactor] 27-27: Too many positional arguments (9/5)
(R0917)
edenai_apis/apis/google/google_video_api.py
[refactor] 929-929: Too many arguments (9/7)
(R0913)
[refactor] 929-929: Too many positional arguments (9/5)
(R0917)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (7)
edenai_apis/apis/google/info.json (1)
1357-1359
: LGTM! Video generation async feature addition follows proper structure.The new
generation_async
feature is properly integrated into the video section with appropriate versioning. The absence of constraints appears intentional for this API.edenai_apis/apis/microsoft/info.json (1)
1769-1773
: LGTM! New video category with generation_async feature properly structured.The addition of the video category follows Microsoft's API structure conventions and uses appropriate versioning with "Azure AI Foundry".
edenai_apis/apis/microsoft/microsoft_api.py (2)
9-9
: LGTM! Video API import properly added.The MicrosoftVideoApi import follows the established pattern for other API mixins.
30-30
: LGTM! Video API mixin correctly integrated.The MicrosoftVideoApi mixin is properly added to the inheritance list, following the same pattern as other API mixins.
edenai_apis/features/video/generation_async/generation_async_args.py (2)
27-27
: LGTM! Updated default text for video generation.The new default text provides a clear and descriptive example for video generation prompts.
37-38
: LGTM! Provider settings added for new video generation capabilities.The Google and Microsoft provider settings are properly configured and align with the API additions in the respective provider files.
edenai_apis/apis/google/google_video_api.py (1)
3-3
: LGTM!The new imports are appropriate for the video generation functionality.
Also applies to: 5-5, 72-74, 88-91
def video__generation_async__launch_job( | ||
self, | ||
text: str, | ||
duration: Optional[int] = 6, | ||
fps: Optional[int] = 24, | ||
dimension: Optional[str] = "1280x720", | ||
seed: Optional[float] = 12, | ||
file: Optional[str] = None, | ||
file_url: Optional[str] = None, | ||
model: Optional[str] = None, | ||
**kwargs, | ||
) -> AsyncLaunchJobResponseType: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove or implement unused parameters.
The following parameters are accepted but not used in the implementation:
dimension
(line 935)fps
(line 934)seed
(line 936)file_url
(line 938)
Either implement these parameters in the request payload or remove them from the method signature to avoid confusion.
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 929-929: Too many arguments (9/7)
(R0913)
[refactor] 929-929: Too many positional arguments (9/5)
(R0917)
🤖 Prompt for AI Agents
In edenai_apis/apis/google/google_video_api.py around lines 929 to 940, the
parameters dimension, fps, seed, and file_url are declared but not used in the
method video__generation_async__launch_job. To fix this, either remove these
unused parameters from the method signature if they are unnecessary, or update
the method implementation to include them appropriately in the request payload
or processing logic to ensure they have an effect.
response = requests.get(uri, headers=headers) | ||
file_content = response.content | ||
base64_encoded_string = base64.b64encode(file_content).decode("utf-8") | ||
resource_url = upload_file_bytes_to_s3( | ||
BytesIO(file_content), ".mp4", USER_PROCESS | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for S3 upload and consider memory usage for large videos.
- The entire video content is loaded into memory which could cause issues with large files.
- No error handling if S3 upload fails - the video data would be lost.
Consider streaming the video content or implementing error handling:
response = requests.get(uri, headers=headers)
+if response.status_code != 200:
+ raise ProviderException(
+ message=f"Failed to download video: {response.status_code}",
+ code=response.status_code
+ )
file_content = response.content
base64_encoded_string = base64.b64encode(file_content).decode("utf-8")
-resource_url = upload_file_bytes_to_s3(
- BytesIO(file_content), ".mp4", USER_PROCESS
-)
+try:
+ resource_url = upload_file_bytes_to_s3(
+ BytesIO(file_content), ".mp4", USER_PROCESS
+ )
+except Exception as exc:
+ raise ProviderException(
+ message=f"Failed to upload video to S3: {str(exc)}",
+ code=500
+ ) from exc
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
response = requests.get(uri, headers=headers) | |
file_content = response.content | |
base64_encoded_string = base64.b64encode(file_content).decode("utf-8") | |
resource_url = upload_file_bytes_to_s3( | |
BytesIO(file_content), ".mp4", USER_PROCESS | |
) | |
response = requests.get(uri, headers=headers) | |
if response.status_code != 200: | |
raise ProviderException( | |
message=f"Failed to download video: {response.status_code}", | |
code=response.status_code | |
) | |
file_content = response.content | |
base64_encoded_string = base64.b64encode(file_content).decode("utf-8") | |
try: | |
resource_url = upload_file_bytes_to_s3( | |
BytesIO(file_content), ".mp4", USER_PROCESS | |
) | |
except Exception as exc: | |
raise ProviderException( | |
message=f"Failed to upload video to S3: {str(exc)}", | |
code=500 | |
) from exc |
🤖 Prompt for AI Agents
In edenai_apis/apis/google/google_video_api.py around lines 1002 to 1007, the
code loads the entire video content into memory and uploads it to S3 without
error handling. To fix this, implement error handling around the S3 upload call
to catch and manage upload failures gracefully. Additionally, refactor the code
to stream the video content instead of loading it fully into memory, for example
by using a streaming request or chunked upload, to reduce memory usage with
large video files.
def video__generation_async__launch_job( | ||
self, | ||
text: str, | ||
duration: Optional[int] = 6, | ||
fps: Optional[int] = 24, | ||
dimension: Optional[str] = "1280x720", | ||
seed: Optional[float] = 12, | ||
file: Optional[str] = None, | ||
file_url: Optional[str] = None, | ||
model: Optional[str] = None, | ||
**kwargs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused parameters to match implementation.
The following parameters are not used in the implementation:
fps
(line 31)seed
(line 33)file
(line 34)file_url
(line 35)
Remove these parameters or implement them in the request payload.
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 27-27: Too many arguments (9/7)
(R0913)
[refactor] 27-27: Too many positional arguments (9/5)
(R0917)
🤖 Prompt for AI Agents
In edenai_apis/apis/microsoft/microsoft_video_api.py between lines 27 and 37,
the parameters fps, seed, file, and file_url are declared but not used in the
function implementation. To fix this, either remove these unused parameters from
the function signature to keep it clean and consistent, or if they are intended
to be part of the video generation request, update the function to include them
properly in the request payload sent to the API.
video_response = requests.get(video_url, headers={"Authorization": api_key}) | ||
base64_encoded_string = base64.b64encode(video_response.content).decode( | ||
"utf-8" | ||
) | ||
resource_url = upload_file_bytes_to_s3( | ||
BytesIO(video_response.content), ".mp4", USER_PROCESS | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for video download and S3 upload.
The code lacks error handling for:
- Failed video download (no status check for video_response)
- S3 upload failures
Apply this fix:
video_response = requests.get(video_url, headers={"Authorization": api_key})
+if video_response.status_code != 200:
+ raise ProviderException(
+ message=f"Failed to download video: {video_response.status_code}",
+ code=video_response.status_code
+ )
base64_encoded_string = base64.b64encode(video_response.content).decode(
"utf-8"
)
-resource_url = upload_file_bytes_to_s3(
- BytesIO(video_response.content), ".mp4", USER_PROCESS
-)
+try:
+ resource_url = upload_file_bytes_to_s3(
+ BytesIO(video_response.content), ".mp4", USER_PROCESS
+ )
+except Exception as exc:
+ raise ProviderException(
+ message=f"Failed to upload video to S3: {str(exc)}",
+ code=500
+ ) from exc
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
video_response = requests.get(video_url, headers={"Authorization": api_key}) | |
base64_encoded_string = base64.b64encode(video_response.content).decode( | |
"utf-8" | |
) | |
resource_url = upload_file_bytes_to_s3( | |
BytesIO(video_response.content), ".mp4", USER_PROCESS | |
) | |
video_response = requests.get(video_url, headers={"Authorization": api_key}) | |
if video_response.status_code != 200: | |
raise ProviderException( | |
message=f"Failed to download video: {video_response.status_code}", | |
code=video_response.status_code | |
) | |
base64_encoded_string = base64.b64encode(video_response.content).decode( | |
"utf-8" | |
) | |
try: | |
resource_url = upload_file_bytes_to_s3( | |
BytesIO(video_response.content), ".mp4", USER_PROCESS | |
) | |
except Exception as exc: | |
raise ProviderException( | |
message=f"Failed to upload video to S3: {str(exc)}", | |
code=500 | |
) from exc |
🤖 Prompt for AI Agents
In edenai_apis/apis/microsoft/microsoft_video_api.py around lines 91 to 97, add
error handling for the video download and S3 upload steps. Check the status code
of video_response after the requests.get call and raise or handle an error if
the download failed. Also, wrap the upload_file_bytes_to_s3 call in a try-except
block to catch and handle any exceptions during the S3 upload, ensuring the
function handles failures gracefully.
Summary by CodeRabbit