kevinjqliu commented on code in PR #707:
URL:
https://github.com/apache/arrow-rs-object-store/pull/707#discussion_r3391536505
##########
Cargo.toml:
##########
@@ -82,13 +83,26 @@ futures-channel = {version = "0.3", features = ["sink"]}
[features]
default = ["fs"]
-cloud = ["serde", "serde_json", "quick-xml", "hyper", "reqwest",
"reqwest/stream", "chrono/serde", "base64", "rand", "ring", "http-body-util",
"form_urlencoded", "serde_urlencoded", "tokio"]
-azure = ["cloud", "httparse"]
+cloud-no-crypto = ["serde", "serde_json", "quick-xml", "hyper", "reqwest",
"reqwest/stream", "chrono/serde", "base64", "rand","http-body-util",
"form_urlencoded", "serde_urlencoded", "tokio"]
Review Comment:
I think we should align the feature naming here with the optional `reqwest`
work in #724.
Feel free to chime in on the discussion here:
https://github.com/apache/arrow-rs-object-store/issues/744
Instead of adding a separate `*-no-crypto` naming pattern, I think `*-base`
can be the common escape hatch for both optional transport and optional crypto:
```text
cloud-base = shared cloud implementation without default reqwest or bundled
crypto
aws-base = cloud-base + AWS/S3-specific implementation deps
azure-base = cloud-base + Azure-specific implementation deps
gcp-base = cloud-base + GCS-specific implementation deps
http-base = cloud-base + HTTP-specific implementation deps
reqwest = built-in reqwest HTTP transport
ring = bundled ring crypto provider
aws-lc-rs = bundled aws-lc-rs crypto provider
cloud = cloud-base + reqwest + default crypto provider
compatibility alias for the old cloud behavior
aws = aws-base + reqwest + default crypto provider
azure = azure-base + reqwest + default crypto provider
gcp = gcp-base + reqwest + default crypto provider
http = http-base + reqwest + default crypto provider
```
That gives users explicit composition:
```toml
# S3 implementation only; user provides HTTP connector and crypto provider
object_store = { default-features = false, features = ["aws-base"] }
# S3 implementation + built-in reqwest, but user controls crypto provider
object_store = { default-features = false, features = ["aws-base",
"reqwest"] }
# S3 implementation + custom HTTP connector, but object_store provides
bundled ring crypto
object_store = { default-features = false, features = ["aws-base", "ring"] }
# S3 implementation + built-in reqwest + explicit aws-lc-rs crypto provider
object_store = { default-features = false, features = ["aws-base",
"reqwest", "aws-lc-rs"] }
```
This keeps the default user path simple:
```toml
object_store = { features = ["aws"] }
```
while making the advanced cases composable.
The remaining compatibility/default decision is whether the
batteries-included features should keep `ring` as the default crypto provider
for compatibility, or intentionally switch to `aws-lc-rs`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
