Skip to content

fix(core): add missing extension assignment#689

Merged
jayzhan211 merged 2 commits into
apache:mainfrom
dentiny:hjiang/add-missing-extension
May 6, 2026
Merged

fix(core): add missing extension assignment#689
jayzhan211 merged 2 commits into
apache:mainfrom
dentiny:hjiang/add-missing-extension

Conversation

@dentiny

@dentiny dentiny commented Apr 9, 2026

Copy link
Copy Markdown
Member

Rationale for this change

Disclaimer: this issues was discovered by opus when I was leverage LLM to read through the code.

This PR adds missing extension through BufWriter single put shutdown path.
Reading through the codebase, it reads to me "attributes", "tags" and "extensions" should all be assigned for PutOptions.

Another example for multipart upload options

let opts = PutMultipartOptions {
attributes: self.attributes.take().unwrap_or_default(),
tags: self.tags.take().unwrap_or_default(),
extensions: self.extensions.take().unwrap_or_default(),

What changes are included in this PR?

This PR passes the missing extension field to PutOption.

Are there any user-facing changes?

No

@crepererum

Copy link
Copy Markdown
Contributor

Is it possible to have a regression test for this?

@dentiny
dentiny force-pushed the hjiang/add-missing-extension branch from 921a8aa to 6420573 Compare April 16, 2026 19:02
@dentiny

dentiny commented Apr 16, 2026

Copy link
Copy Markdown
Member Author

Is it possible to have a regression test for this?

Hmmm I think it's not very straightforward; different from attributes, extensions won't appear in the response.

@dentiny

dentiny commented Apr 22, 2026

Copy link
Copy Markdown
Member Author

error: explicit call to .into_iter() in function argument accepting IntoIterator
--> src/client/list.rs:120:36
|
120 | common_prefixes.extend(response.common_prefixes.into_iter());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: this parameter accepts any IntoIterator, so you don't need to call .into_iter()

hmmm I might be on an old toolchain version

@dentiny

dentiny commented May 6, 2026

Copy link
Copy Markdown
Member Author

Hi @crepererum , sorry for the interruption and thanks for the previous reviews, I'm wondering if you mind taking another look maybe when you have some time? Thank you so much!

@jayzhan211
jayzhan211 merged commit e0fbe49 into apache:main May 6, 2026
9 checks passed
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