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

fix: Correct the GET authentication for S3 using rusoto #471

Closed
wants to merge 32 commits into from

Conversation

Marwes
Copy link
Contributor

@Marwes Marwes commented Jul 5, 2019

For some unknown reason authenticated GET accesses fails under certain
configurations. So far I have been unable to determine the exact cause
but after adding rusoto_s3 to deal with all communication to S3 it
fixed itself so at least for my case this would be a good fix.

I wasn't sure exactly what to do about the config file format as that probably needs to change to better fit what rusoto accepts. At least it should be expanded to support reading in the region which currently only seems to work through an environment variable.

Based on #470 since that fixes some build errors for me.

Closes #337

@Marwes Marwes force-pushed the rusoto branch 3 times, most recently from 9c766e1 to 6cf2ac6 Compare August 23, 2019 13:14
@Marwes
Copy link
Contributor Author

Marwes commented Aug 23, 2019

Rebased and moved up contained all accidental formatting into the first commit (see #502 ). https://github.com/mozilla/sccache/pull/471/files/dfe5f403dde8f56c24dd2a03acafa6cfaa378dcc..6cf2ac66baba83ebc28a97ba0e24820bf91e634d gives the actual changes which should be much simpler to review.

@Marwes Marwes mentioned this pull request Aug 23, 2019
Markus Westerlind added 8 commits August 27, 2019 11:38
For some unknown reason authenticated GET accesses fails under certain
configurations. So far I have been unable to determine the exact cause
but after adding `rusoto_s3` to deal with all communication to S3 it
fixed itself so at least for my case this would be a good fix.
@scooter-dangle
Copy link

@Marwes, have you tried to reproduce the issue after #490 was merged? After that change, I no longer see the S3 authentication issue that this PR solved.

@Marwes Marwes closed this Dec 9, 2019
@Marwes
Copy link
Contributor Author

Marwes commented Dec 9, 2019

Seems to work with #490 . Either way this has bit rotted to the point that another, fresher PR has taken its place #522

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.

None yet

2 participants