-
Notifications
You must be signed in to change notification settings - Fork 627
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
Added support for Google buckets storage. #172
Conversation
Named the new project |
if (e.HttpStatusCode != HttpStatusCode.NotFound) | ||
throw; | ||
|
||
await storage.UploadObjectAsync(_bucketName, path, contentType, content, cancellationToken: cancellationToken); |
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.
Does this throw a GoogleApiException
if the object already exists? If so, you should catch that, get a hash of the already existing object, and return AlreadyExists
if the object has the same content or Conflict
if the object has different content. See the Azure Blob Storage code for an example: https://github.com/loic-sharma/BaGet/blob/master/src/BaGet.Azure/BlobStorageService.cs#L68
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.
No, UploadObjectAsync
silently overwrites. It has a UploadObjectOptions
parameter but setting its IfGenerationNotMatch
property only seems to silently overwrite or ignore, you're not told what happened. I instead opted for using the path as "key", if the package already exists youre not allowed to overwrite, regardless of if the content has changed. Is that acceptable?
It still builds, I dont remember how these were added.
{ | ||
try | ||
{ | ||
await storage.GetObjectAsync(_bucketName, CoercePath(path), cancellationToken: cancellationToken); |
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.
This isn't quite right. The steps should be something like:
- Attempt to
UploadObjectAsync
- If the upload fails because the file exists (likely a
HttpStatusCode.Conflict
, but please verify this as I've never used GCP), then:- Download the object
- Return
PutResult.AlreadyExists
if the object is the same ascontent
, orPutResult.Conflict
otherwise.
For an example of this, please see Azure's BlobStorageService
This addresses code review feedback at loic-sharma#172 (comment).
This addresses code review feedback at loic-sharma#172 (comment).
This addresses code review feedback at loic-sharma#172 (comment).
@loic-sharma This has been superseded by #233 and can be closed. |
Yes it can, thanks for the reminder! :) |
What does this PR do?
Adds support for Google buckets storage.
Motivation
Needed for persistent storage.