pants.ci.toml

[GLOBAL]
colors = true
remote_provider = "experimental-github-actions-cache"
remote_cache_read = true
remote_cache_write = true
remote_instance_name = "pants_ci_cache"

[stats]
log = true

Thanks for filing an issue. It looks like apache/opendal#3985 is a fix for S3-backed GHAC, but that only landed in opendal 0.44.2. Pants currently uses 0.41.0.

So, maybe a fix here is to update the version of OpenDAL. @DLukeNelson, would you like to contribute that fix?

I'd love to! I'll open up a PR right away

#21779

@huonw Unfortunately, our above changes were not enough. The new error I'm seeing in CI is like this:

Failed to read from remote cache (1 occurrences so far): failed to read pants_ci_cache/action-cache/1f/62/1f6292e82842e0bc4146825c85f31299878c1c05bf26ee2fa15fea4764bdfab8: Unexpected (temporary) at read, context: { url: https://<github_url>/_services/artifactcache/ocrHE8ZScyxq8pTiT1IyKRHNwdDtYL7HOvJRB2oXuk7JCGeFXS/_apis/artifactcache/cache?keys=pants_ci_cache/action-cache/1f/62/1f6292e82842e0bc4146825c85f31299878c1c05bf26ee2fa15fea4764bdfab8&version=pants-1, called: http_util::Client::send, service: ghac, path: pants_ci_cache/action-cache/1f/62/1f6292e82842e0bc4146825c85f31299878c1c05bf26ee2fa15fea4764bdfab8, range: 0- } => send http request, source: error sending request for url (https://<github_url>/_services/artifactcache/ocrHE8ZScyxq8pTiT1IyKRHNwdDtYL7HOvJRB2oXuk7JCGeFXS/_apis/artifactcache/cache?keys=pants_ci_cache/action-cache/1f/62/1f6292e82842e0bc4146825c85f31299878c1c05bf26ee2fa15fea4764bdfab8&version=pants-1): error trying to connect: invalid URL, scheme is not http

any thoughts?

Hm, that error message looks like it comes from the hyper library (https://github.com/hyperium/hyper-util/blob/5f055e36d6f945b51b4d4b023290f551ae7eb4b6/src/client/legacy/connect/http.rs#L392), which is used by OpenDAL (Pants -> OpenDAL -> Reqwest -> Hyper). It looks like the default setting of that library is to only support HTTP, no HTTPS.

Maybe this is a bug in OpenDAL and its usage of Reqwest?

I did find this: apache/opendal#5205
which seems to be the same set of errors that we have. (and we too have disabled all default features from opendal. I'm not sure I understand the ask in that thread though. (to enable reqwest's https support).

Pants' Cargo.toml does have reqwest = { version = "0.12", default-features = false, features = ["stream", "rustls-tls"] } which seems like tls/https ought to be enabled.

Hi, pants is using opendal 0.45.1 now: https://github.com/pantsbuild/pants/blob/b9587acfb8c2f6323cd00b4ec88e2a487a758114/src/rust/engine/Cargo.toml#L278C1-L282

But opendal 0.45.1 is relaying on reqwest 0.11 instead of 0.12: https://crates.io/crates/opendal/0.45.1/dependencies

So specify reqwest = { version = "0.12", default-features = false, features = ["stream", "rustls-tls"] } doesn't work.

To get this fixed, we should either:

• upgrade opendal to latest (recommanded)
• specify reqwest 0.11 in cargo.toml

I'm unsure if pants may be relying on reqwest for any other reason, so its probably best to upgrade opendal to latest. (The earlier PR in this thread attempted this, but ran into some trouble with breaking changes, so I cut the scope and only upgraded to 0.45.1)

Since we now know that this is insufficient, we ought to fully upgrade. @huonw do you agree?

Thanks for the analysis @Xuanwo, very helpful! (And thanks for maintaining OpenDAL!)

I've posted #21835 with the full upgrade, let's see what CI thinks of it!

upgrade opendal to latest (recommanded)

As a bit of feedback from your users, we initially tried this in #21779, but found it was quite difficult due to breaking changes with non-obvious resolutions (e.g. not mentioned or explained in the changelog).

For instance, I ended up having to do a full git bisect on the OpenDAL code itself to work out what was causing a behaviour change and thus deduce a resolution (#21779 (review)).

As a bit of feedback from your users, we initially tried this in #21779, but found it was quite difficult due to breaking changes with non-obvious resolutions (e.g. not mentioned or explained in the changelog).

Hi, @huonw, sorry about that. I tried my best to split the changes across several versions and implement them gradually, but it becomes frustrating when multiple breaking changes appear simultaneously. We’ve maintained an upgrade guide for each version, which I believe will be helpful: https://docs.rs/opendal/latest/opendal/docs/upgrade/index.html.

I’m more than happy to help address the breaking changes introduced by OpenDAL. I expect that users who only use the public API (not touching the opendal::raw mods) should find it easy to upgrade. This is also an opportunity for me to validate my design and the upgrade path.

Ah cool, I had not seen that page, nor had I noticed there's more specific upgrade contents available in the GitHub releases: https://github.com/apache/opendal/releases

Is it easy to have each section in the CHANGELOG file include a bit of text like See [the GitHub release](...) for how to upgrade or similar? (Or some link to the upgrade page too)

I’m more than happy to help address the breaking changes introduced by OpenDAL

All good, I think I've resolved them in #21835.

Ah cool, I had not seen that page, nor had I noticed there's more specific upgrade contents available in the GitHub releases: apache/opendal/releases

Is it easy to have each section in the CHANGELOG file include a bit of text like See [the GitHub release](...) for how to upgrade or similar? (Or some link to the upgrade page too)

Sure, I'll ensure they're included in the changelog.

All good, I think I've resolved them in #21835.

🚀