-
Notifications
You must be signed in to change notification settings - Fork 644
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
GetEtagAsync #6577
GetEtagAsync #6577
Conversation
string folderName, | ||
string fileName) | ||
{ | ||
return await Task.FromResult<string>(null); |
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.
It would be clearer to throw NotImplementedException.
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.
Updated.
fileName = fileName ?? throw new ArgumentNullException(nameof(fileName)); | ||
|
||
var container = await GetContainerAsync(folderName); | ||
var blob = container.GetBlobReference(fileName); |
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.
When would this return null?
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.
If the blob doesn't exist, FetchAttributesAsync
will throw. If the intended behavior is to return null if the blob doesn't exist, I'd rename the method. Maybe something like GetETagOrNullAsync
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.
Updated the name and also to catch the case of FetchAttributesAsync throwing.
@@ -426,6 +426,24 @@ public async Task<Uri> GetFileReadUriAsync(string folderName, string fileName, D | |||
} | |||
} | |||
|
|||
public async Task<string> GetETagAsync( |
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.
Don't we need a method that calls this in the package file service?
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.
I would not add it until needed.
Add an api for reading the etag for an azure blob.