Skip to content

Implement AppendObject() API#2082

Merged
harshavardhana merged 1 commit intominio:masterfrom
harshavardhana:append-object
Apr 2, 2025
Merged

Implement AppendObject() API#2082
harshavardhana merged 1 commit intominio:masterfrom
harshavardhana:append-object

Conversation

@harshavardhana
Copy link
Copy Markdown
Member

No description provided.

@harshavardhana harshavardhana force-pushed the append-object branch 2 times, most recently from 1686737 to 9b86efc Compare March 25, 2025 21:23
Copy link
Copy Markdown
Member

@HJLebbink HJLebbink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, works as advertised

Copy link
Copy Markdown
Contributor

@klauspost klauspost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing major.

Comment thread api-append-object.go Outdated
Comment thread api-append-object.go Outdated
Comment thread api-append-object.go Outdated
Comment thread api-append-object.go
Comment thread api-append-object.go Outdated
Comment thread api-append-object.go
Comment thread api-append-object.go
@harshavardhana harshavardhana force-pushed the append-object branch 4 times, most recently from ac12297 to 728e2ce Compare March 27, 2025 06:01
@harshavardhana
Copy link
Copy Markdown
Member Author

This PR fixes the checksum behavior. @klauspost - I also added a few missing things in the checksum behavior as per AWS S3 behavior.

Server side is also changing in the Append() PR itself.

Copy link
Copy Markdown
Contributor

@klauspost klauspost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<3

@harshavardhana harshavardhana merged commit 6c30280 into minio:master Apr 2, 2025
5 checks passed
@harshavardhana harshavardhana deleted the append-object branch April 2, 2025 03:42
project-mirrors-bot-tu bot pushed a commit to project-mirrors/forgejo that referenced this pull request Apr 9, 2025
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [github.com/minio/minio-go/v7](https://github.com/minio/minio-go) | require | patch | `v7.0.88` -> `v7.0.90` |

---

### Release Notes

<details>
<summary>minio/minio-go (github.com/minio/minio-go/v7)</summary>

### [`v7.0.90`](https://github.com/minio/minio-go/releases/tag/v7.0.90): Bugfix v7.0.90

[Compare Source](minio/minio-go@v7.0.89...v7.0.90)

#### What's Changed

-   Add anonymous chunked encoding with trailing headers by [@&#8203;klauspost](https://github.com/klauspost) in minio/minio-go#2084
-   Implement AppendObject() API by [@&#8203;harshavardhana](https://github.com/harshavardhana) in minio/minio-go#2082
-   Update x/net version by [@&#8203;BorjaOuterelo](https://github.com/BorjaOuterelo) in minio/minio-go#2085
-   Rety iterators by [@&#8203;tlyons-cs](https://github.com/tlyons-cs) in minio/minio-go#2087
-   Add function for getting creds of remote client by [@&#8203;shtripat](https://github.com/shtripat) in minio/minio-go#2089

#### New Contributors

-   [@&#8203;BorjaOuterelo](https://github.com/BorjaOuterelo) made their first contribution in minio/minio-go#2085
-   [@&#8203;tlyons-cs](https://github.com/tlyons-cs) made their first contribution in minio/minio-go#2087

**Full Changelog**: minio/minio-go@v7.0.89...v7.0.90

### [`v7.0.89`](https://github.com/minio/minio-go/releases/tag/v7.0.89): Bugfix Release

[Compare Source](minio/minio-go@v7.0.88...v7.0.89)

#### What's Changed

-   add PurgeOnDelete to versioning config by [@&#8203;poornas](https://github.com/poornas) in minio/minio-go#2074
-   Adds `TokenRevokeType` field to credential providers. by [@&#8203;taran-p](https://github.com/taran-p) in minio/minio-go#2075
-   make downtime info as map to denote per target info by [@&#8203;Praveenrajmani](https://github.com/Praveenrajmani) in minio/minio-go#2079
-   update deps and move CI/CD to go1.23, go1.24 by [@&#8203;harshavardhana](https://github.com/harshavardhana) in minio/minio-go#2080
-   update golint version by [@&#8203;harshavardhana](https://github.com/harshavardhana) in minio/minio-go#2083

#### New Contributors

-   [@&#8203;taran-p](https://github.com/taran-p) made their first contribution in minio/minio-go#2075

**Full Changelog**: minio/minio-go@v7.0.88...v7.0.89

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "* 0-3 * * *" (UTC), Automerge - "* 0-3 * * *" (UTC).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4yMzMuNSIsInVwZGF0ZWRJblZlciI6IjM5LjIzMy41IiwidGFyZ2V0QnJhbmNoIjoiZm9yZ2VqbyIsImxhYmVscyI6WyJkZXBlbmRlbmN5LXVwZ3JhZGUiLCJ0ZXN0L25vdC1uZWVkZWQiXX0=-->

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7495
Reviewed-by: Earl Warren <[email protected]>
Co-authored-by: Renovate Bot <[email protected]>
Co-committed-by: Renovate Bot <[email protected]>
Comment thread hook-reader.go
func newHook(source, hook io.Reader) io.Reader {
if hook == nil {
return &hookReader{source: source}
return source
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change cause us headaches. because of this change minio is closing the passed reader.

https://codeberg.org/forgejo/forgejo/issues/8529

so if you pass a io.File to the PutObject function, then minio will close it.

minio-go/api.go

Line 681 in 456f9b2

defer bodyCloser.Close()

This is a breaking api behavior

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go http client closes the body, that is exactly what we do in this SDK. What is the problem?

Copy link
Copy Markdown
Contributor

@klauspost klauspost Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harshavardhana It is a bit "bad manner" to close the body - especially since you aren't given an io.ReadCloser, but an io.Reader you wouldn't expect it to be closed.

I guess the wrapper prevented that from happening before.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem is that' it's not documented and changed in a patch release. it's also not consistent for all call sites.

klauspost added a commit to klauspost/minio-go that referenced this pull request Dec 10, 2025
`ChecksumMode` was probably a mistake to add in minio#2082 - but now we've committed to it.

It must however match this: https://github.com/miniohq/eos/blob/cc3c249d4ecaeb26d503fd264c9fd7df72b0b0d2/cmd/api-response.go#L380

Otherwise it is not picked up as metadata in list responses.
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.

4 participants