Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: bump opendal to 0.34.0 #1760

Merged
merged 2 commits into from
May 10, 2023

Conversation

saiintbrisson
Copy link
Contributor

@saiintbrisson saiintbrisson commented May 9, 2023

OpenDAL 0.34.0 comes with a nicer way to build GCS caches and
a bump to reqsign 0.10.1, which means Google's External Accounts
are now supported, making it easier to use Sccache on build pipelines
counting on Workload Identity Federation for their auth.

It's now necessary to define the SCCACHE_REGION when using AWS S3
with the default endpoint.

@sylvestre
Copy link
Collaborator

it would be nice to update the doc
(and tests if possible)

@Xuanwo
Copy link
Collaborator

Xuanwo commented May 9, 2023

Thanks!

@saiintbrisson
Copy link
Contributor Author

@sylvestre would it be a problem to link to an (official) external resource for the external account as an example? https://cloud.google.com/blog/products/identity-security/enabling-keyless-authentication-from-github-actions

@sylvestre
Copy link
Collaborator

Both would be nice :)

OpenDAL 0.34.0 comes with a nicer way to build GCS caches and
a bump to `reqsign 0.10.1`, which means Google's External Accounts
are now supported, making it easier to use Sccache on build pipelines
counting on Workload Identity Federation for their auth.
@saiintbrisson saiintbrisson force-pushed the chore/bump-opendal-and-reqsign branch from 8dcd674 to 44a35dc Compare May 9, 2023 16:38
@saiintbrisson
Copy link
Contributor Author

@sylvestre PTAL. I've made some changes to the GCS docs ordering to make it easier to find info on credentials

@codecov-commenter
Copy link

codecov-commenter commented May 9, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.07 ⚠️

Comparison is base (3e1717e) 29.40% compared to head (a7c330b) 29.33%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1760      +/-   ##
==========================================
- Coverage   29.40%   29.33%   -0.07%     
==========================================
  Files          49       49              
  Lines       17622    17620       -2     
  Branches     8496     8516      +20     
==========================================
- Hits         5181     5169      -12     
+ Misses       7322     7316       -6     
- Partials     5119     5135      +16     
Impacted Files Coverage Δ
src/cache/gcs.rs 10.63% <0.00%> (+0.83%) ⬆️
src/cache/s3.rs 55.35% <0.00%> (ø)
tests/sccache_args.rs 36.58% <ø> (ø)

... and 13 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@saiintbrisson saiintbrisson force-pushed the chore/bump-opendal-and-reqsign branch from 44a35dc to 68a0cbe Compare May 9, 2023 17:49
@saiintbrisson saiintbrisson force-pushed the chore/bump-opendal-and-reqsign branch from 68a0cbe to a7c330b Compare May 9, 2023 17:56
@saiintbrisson saiintbrisson requested a review from Xuanwo May 9, 2023 17:58
Copy link
Collaborator

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice docs!

@Xuanwo
Copy link
Collaborator

Xuanwo commented May 10, 2023

cc @sylvestre to take an another round of review.

@sylvestre sylvestre merged commit 55b4e58 into mozilla:main May 10, 2023
@sylvestre
Copy link
Collaborator

thanks!

@saiintbrisson saiintbrisson deleted the chore/bump-opendal-and-reqsign branch May 10, 2023 16:29
@messense
Copy link
Contributor

messense commented May 23, 2023

Hi @sylvestre can you release a new version with this patch? sccache 0.4.2 depends on older opendal that uses ureq with manual proxy configuration instead of reqwest, which doesn't support NO_PROXY env var. Thanks!

@sylvestre
Copy link
Collaborator

sure, building

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.

5 participants