-
Notifications
You must be signed in to change notification settings - Fork 471
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
Use signed url for local sync #1013
Conversation
WalkthroughThis changeset introduces a more secure way of handling file syncing by generating temporal signed URLs. The Changes
Uplevel your code reviews with Entelligence.AI ProEntelligence.AI ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, Entelligence.AI Pro is free for open source projects. |
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.
Files selected (2)
- backend/routers/sync.py (2)
- backend/utils/other/storage.py (1)
Review comments generated (4)
- Review: 1
- LGTM: 3
Tips
Chat with Entelligence.AI Bot (@Entelligence.AI
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@Entelligence.AI
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@Entelligence.AI: ignore
anywhere in the PR description to pause further reviews from the bot.
def get_syncing_file_temporal_signed_url(file_path: str): | ||
bucket = storage_client.bucket(syncing_local_bucket) | ||
blob = bucket.blob(file_path) | ||
blob.upload_from_filename(file_path) | ||
return _get_signed_url(blob, 15) |
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.
The get_syncing_file_temporal_signed_url
function uploads a file to the bucket and then returns a signed URL for that file. However, it seems like this function is supposed to generate a signed URL for an existing file in the bucket, not upload a new one. The line blob.upload_from_filename(file_path)
should probably be removed.
- blob.upload_from_filename(file_path)
return _get_signed_url(blob, 15)
Also, there's no error handling here. What if the file doesn't exist in the bucket? You should add some error handling to check if the blob exists before trying to get its signed URL.
if not blob.exists():
raise FileNotFoundError(f"The file {file_path} does not exist in the bucket.")
return _get_signed_url(blob, 15)
Summary by Entelligence.AI