You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/03/04 21:11:32 UTC
[GitHub] [arrow-datafusion] alamb opened a new pull request, #5478: Minor: Move `ObjectStoreRegistry` to datafusion_execution crate
alamb opened a new pull request, #5478:
URL: https://github.com/apache/arrow-datafusion/pull/5478
# Which issue does this PR close?
Part of https://github.com/apache/arrow-datafusion/issues/1754
# Rationale for this change
I am trying to extract the physical_plan code into its own crate; and to do so I need to remove the circular dependencies between core --> datasource --> execution --> datasource
The `ObjectStoreRegistry` is used at runtime, so move it into the datafusion_execution crate
See more details in https://github.com/apache/arrow-datafusion/issues/1754#issuecomment-1452438453
# What changes are included in this PR?
1. Move Move `ObjectStoreRegistry` to datafusion_execution crate
2. Separate some tests that mixed ListingTableUrl (part of datasource) and ObjectStoreRegistry (part of execution)
# Are these changes tested?
Covered by existing tests
# Are there any user-facing changes?
No, I added a `pub use` so this code movement should not affect users
--
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
[GitHub] [arrow-datafusion] alamb merged pull request #5478: Minor: Move `ObjectStoreRegistry` to datafusion_execution crate
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #5478:
URL: https://github.com/apache/arrow-datafusion/pull/5478
--
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
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #5478: Minor: Move `ObjectStoreRegistry` to datafusion_execution crate
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5478:
URL: https://github.com/apache/arrow-datafusion/pull/5478#discussion_r1125560691
##########
datafusion/core/src/datasource/listing/mod.rs:
##########
@@ -103,3 +103,52 @@ impl From<ObjectMeta> for PartitionedFile {
}
}
}
+
+#[cfg(test)]
+mod tests {
Review Comment:
these tests are of `ListingTableUrl` which is in core/src/datasource/listing I moved them here
--
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
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #5478: Minor: Move `ObjectStoreRegistry` to datafusion_execution crate
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5478:
URL: https://github.com/apache/arrow-datafusion/pull/5478#discussion_r1125673857
##########
datafusion/core/src/execution/runtime_env.rs:
##########
@@ -26,9 +26,11 @@ use std::collections::HashMap;
use crate::datasource::datasource::TableProviderFactory;
use crate::datasource::listing_table_factory::ListingTableFactory;
-use crate::datasource::object_store::ObjectStoreRegistry;
Review Comment:
the point of this PR is to remove the dependency here on `datasource`
The other references to datasource are removed in https://github.com/apache/arrow-datafusion/pull/5477
--
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
[GitHub] [arrow-datafusion] ursabot commented on pull request #5478: Minor: Move `ObjectStoreRegistry` to datafusion_execution crate
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #5478:
URL: https://github.com/apache/arrow-datafusion/pull/5478#issuecomment-1462565682
Benchmark runs are scheduled for baseline = 07b07d52fb04e324bedd145fb53b217cb7fab9fe and contender = 55c7c4d8e9d6f64314460ba7580e36dbce140594. 55c7c4d8e9d6f64314460ba7580e36dbce140594 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/e38bb9b0a3b34774bc17eb7db27ea8e6...4e1b451ab366428cb982e977577a7335/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/3d902f1e2d454f96b72a4cf26075e690...c053f212835c44a5aec8d8cee67cd2d5/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/cfe16da006f2435097af5e523b072761...8b1df4c910174688b66f955067cd878f/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/3f94473e73b9441dbd81aa2c32214c5a...896c0f76b2e14b55919ad34bb59ba40d/)
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #5478: Minor: Move `ObjectStoreRegistry` to datafusion_execution crate
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5478:
URL: https://github.com/apache/arrow-datafusion/pull/5478#discussion_r1125560703
##########
datafusion/execution/src/object_store.rs:
##########
@@ -255,34 +245,4 @@ mod tests {
ObjectStoreUrl::parse("s3://username:password@host:123/foo").unwrap_err();
assert_eq!(err.to_string(), "Execution error: ObjectStoreUrl must only contain scheme and authority, got: /foo");
}
-
Review Comment:
Moved this into the datasource/listing module
--
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