Skip to content

V5.0.0 patch release for a customer - #1193

Merged
harshavardhana merged 4 commits into
minio:5.xfrom
ebozduman:v5.0.0-patch
Oct 16, 2024
Merged

harshavardhana merged 4 commits into
minio:5.xfrom
ebozduman:v5.0.0-patch

Conversation

@ebozduman

Copy link
Copy Markdown
Contributor

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.

Comment thread Minio/MinioClient.cs Outdated
// 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;

Copy link
Copy Markdown
Collaborator

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.

@ebozduman ebozduman Oct 16, 2024 •

Copy link
Copy Markdown
Contributor Author

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.

@ramondeklein

Copy link
Copy Markdown
Collaborator

Are we planning to deploy a new v5 SDK version?

@ebozduman

Copy link
Copy Markdown
Contributor Author

Yes.
The decision is to create a patch release for the customer and the only requirement the customer asked for is to support zero-sized files/objects.

@ramondeklein

Copy link
Copy Markdown
Collaborator

@ebozduman If they can do non-zero size files using this method, then I'm fine with that.

@harshavardhana
harshavardhana merged commit c1f7a79 into minio:5.x Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants