-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix: make google storage not required #4321
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
base: main
Are you sure you want to change the base?
Conversation
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.
Code Review
This pull request aims to make Google Cloud Storage optional and improve error handling by catching Forbidden exceptions. The changes are in the right direction, but the try...except block in get_user_has_speech_profile is incomplete. It doesn't cover all API calls that could raise a Forbidden exception, potentially leaving an unhandled exception. I've left a critical comment with a suggestion to make the error handling more robust.
|
@gemini-code-assist fixed. re-review |
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.
Code Review
This pull request effectively addresses an issue where a missing or misconfigured Google Cloud Storage billing account would cause transcription to fail. The changes introduced in backend/utils/other/storage.py are straightforward and correct. By adding a check for the speech_profiles_bucket configuration and handling Forbidden exceptions, the get_user_has_speech_profile function is now more robust and will gracefully return False in these scenarios. This makes the Google Storage dependency optional as intended. The implementation is clean and I have no concerns. Good job!
Love you gemini :) |
Seems like a recent commit made a google storage billing account necessary, else transcription won't work.
This very simple fix changes that.
Error handling improvements:
Forbiddenexception fromgoogle.cloud.exceptionsfor more granular error handling.get_user_has_speech_profileto immediately returnFalseif thespeech_profiles_bucketis not set, preventing unnecessary operations.try/exceptblock to catchForbiddenexceptions when checking if a speech profile blob exists, ensuring the function returnsFalseif access is denied.