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

Remove need for AWS credentials #130

Merged
merged 3 commits into from
Nov 27, 2023
Merged

Remove need for AWS credentials #130

merged 3 commits into from
Nov 27, 2023

Conversation

ejm714
Copy link
Collaborator

@ejm714 ejm714 commented Nov 27, 2023

Small change to set no-sign-request to True for downloading the landcover map so that a user does not need to have AWS credentials configured.

Copy link

render bot commented Nov 27, 2023

@ejm714 ejm714 requested a review from pjbull November 27, 2023 17:16
Copy link

codecov bot commented Nov 27, 2023

Codecov Report

Merging #130 (7500a8c) into main (ebdf47e) will not change coverage.
The diff coverage is 50.0%.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #130   +/-   ##
=====================================
  Coverage   75.0%   75.0%           
=====================================
  Files         12      12           
  Lines        860     860           
=====================================
  Hits         645     645           
  Misses       215     215           
Files Coverage Δ
cyfi/data/features.py 50.0% <50.0%> (ø)

@ejm714
Copy link
Collaborator Author

ejm714 commented Nov 27, 2023

@pjbull I looked at the uses of AnyPath and corrected one to Path which was a type hint. The other uses of AnyPath are in the (unsupported) experiment module and these make sense since it's reasonable to let people / us use private buckets for csvs for those.

@pjbull
Copy link
Member

pjbull commented Nov 27, 2023

@ejm714 ok looks great! Just confirming that you tested this with your creds file moved and ensured it worked as expected?

@ejm714
Copy link
Collaborator Author

ejm714 commented Nov 27, 2023

Just confirming that you tested this with your creds file moved and ensured it worked as expected?

Yep!

@ejm714 ejm714 merged commit ba7f54e into main Nov 27, 2023
9 of 10 checks passed
@ejm714 ejm714 deleted the no-sign-request branch November 27, 2023 19:32
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.

2 participants