You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "netthier (via GitHub)" <gi...@apache.org> on 2024/04/25 14:45:19 UTC
[PR] Add `BufWriter::with_attributes` [arrow-rs]
netthier opened a new pull request, #5693:
URL: https://github.com/apache/arrow-rs/pull/5693
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #5692.
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
This PR adds the method `with_attributes` to `BufWriter`, which allows users to set attributes on uploaded files.
I originally intended to add `with_tags` as well, as both `put_opts` and `put_multipart_opts` support them, but I did not know how to retrieve tags after setting them, leaving me unable to test that method.
# Are there any user-facing changes?
Yes, this PR introduces a new method on `BufWriter`. There are no breaking changes though.
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking change` label.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add `BufWriter::with_attributes` [arrow-rs]
Posted by "netthier (via GitHub)" <gi...@apache.org>.
netthier commented on PR #5693:
URL: https://github.com/apache/arrow-rs/pull/5693#issuecomment-2077708433
Could you maybe elaborate how that hooking into could look like? If it's not too difficult I'd like to include it in this PR for completeness.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add `BufWriter::with_attributes` [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #5693:
URL: https://github.com/apache/arrow-rs/pull/5693#issuecomment-2077716548
https://github.com/apache/arrow-rs/blob/master/object_store/src/lib.rs#L2362 is an integration test called by both the AWS and Azure test suites, and is provided with a method for retrieving the tags for a given path.
It should just be a case of extending this integration test with your new BufWriter functionality and that should do the trick
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add `BufWriter::with_attributes` and `::with_tags` in `object_store` [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold merged PR #5693:
URL: https://github.com/apache/arrow-rs/pull/5693
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add `BufWriter::with_attributes` [arrow-rs]
Posted by "netthier (via GitHub)" <gi...@apache.org>.
netthier commented on PR #5693:
URL: https://github.com/apache/arrow-rs/pull/5693#issuecomment-2077740402
I'm trying to add `BufWriter` to `tagging`, but I can't figure out how to get the `Arc<dyn ObjectStore>` required. `tagging` takes a `&dyn ObjectStore`, and changing the method signature so that the caller needs to pass an `Arc` leads to more issues...
Any idea what I could do?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add `BufWriter::with_attributes` and `::with_tags` in `object_store` [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #5693:
URL: https://github.com/apache/arrow-rs/pull/5693#discussion_r1580864302
##########
object_store/src/aws/mod.rs:
##########
@@ -437,10 +437,16 @@ mod tests {
// Object tagging is not supported by S3 Express One Zone
if config.session_provider.is_none() {
- tagging(&integration, !config.disable_tagging, |p| {
- let client = Arc::clone(&integration.client);
- async move { client.get_object_tagging(&p).await }
- })
+ tagging(
Review Comment:
So one way you could avoid this would be to do something like
```
let integration = Arc::new(config.build().unwrap());
let config = &integration.client.config;
let test_not_exists = config.copy_if_not_exists.is_some();
let test_conditional_put = config.conditional_put.is_some();
put_get_delete_list(&*integration).await;
get_opts(&*integration).await;
list_uses_directories_correctly(&*integration).await;
list_with_delimiter(&*integration).await;
rename_and_copy(&*integration).await;
stream_get(&*integration).await;
multipart(&*integration, &*integration).await;
signing(&*integration).await;
s3_encryption(&*integration).await;
put_get_attributes(&*integration).await;
// Object tagging is not supported by S3 Express One Zone
if config.session_provider.is_none() {
tagging(Arc::clone(&integration) as _, !config.disable_tagging, |p| {
let client = Arc::clone(&integration.client);
async move { client.get_object_tagging(&p).await }
})
.await;
}
```
But I am not sure it is really cleaner.
FWIW I did try to push this but I don't seem to have the ability to edit your PRs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add `BufWriter::with_attributes` [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #5693:
URL: https://github.com/apache/arrow-rs/pull/5693#issuecomment-2077699480
> how to retrieve tags after setting them, leaving me unable to test that method.
We don't expose it publicly, as there is no consistency between stores, but there is an integration test of object tagging that you could hook into
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org