-
Notifications
You must be signed in to change notification settings - Fork 230
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
V5.0.0 patch release for a customer #1193
Conversation
@@ -1839,7 +1839,7 @@ internal override void Validate() | |||
if (!string.IsNullOrWhiteSpace(FileName)) Utils.ValidateFile(FileName); | |||
// Check object size when using stream data | |||
if (ObjectStreamData is not null && ObjectSize == 0) | |||
throw new ArgumentException($"{nameof(ObjectSize)} must be set"); | |||
ObjectSize = ObjectStreamData.Length; |
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 don't think this is what the client asked for. The client asked that if he specified ObjectSize = 0
that it should create an empty file. Now it won't use the ObjectSize
parameter, but it will auto-detect the size from the stream (note that this is only possible for streams that have their CanSeek
property set).
I think we should initialize ObjectSize
to -1
in the constructor and that would mean that the size will be auto-detected. When set to 0 or larger, then it should use that size instead. We should also ensure that we don't read more than is specified. So when ObjectSize = 10
and the stream delivers 20 bytes, then we should only upload 10 bytes.
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.
Makes sense.
I was thinking more towards like ObjectSize
being useless as it is implemented right now and maybe replacing it with auto-detect object size could be a better idea since retrieving part of a stream/file may not be always possible.
But what you've suggested makes much more sense.
On the other side, the code was like this quite for some time.
I'll make the change here to check for -1
instead of 0
to correct this part.
However, to make it easier to generate a 5.0.0 patch release package available for the customer, I suggest to create a separate issue for it and address it after the patch release.
Please share your opinion.
Are we planning to deploy a new v5 SDK version? |
Yes. |
@ebozduman If they can do non-zero size files using this method, then I'm fine with that. |
A customer complained about PutObjectAsync api not supporting zero sized objects.
The customer uses version 5.0.0, so the patch fix is created on version 5.0.0.