Skip to content

Add GCS Getter #170

Merged
azr merged 18 commits intomasterfrom
unknown repository
Mar 6, 2019
Merged

Add GCS Getter #170
azr merged 18 commits intomasterfrom
unknown repository

Conversation

@lifangmoler
Copy link
Copy Markdown
Contributor

@lifangmoler lifangmoler commented Mar 1, 2019

Why?

Add gcs (google cloud storage) as another protocol to download files/directories like S3, Git and others.

In order to pass the integration tests:

  1. Need to add Google Credentials inside get_gcs_test.go
  2. Create gcs buckets and folder/files( for consistency, I created a bucket with the same name to S3)

@hashicorp-cla
Copy link
Copy Markdown

hashicorp-cla commented Mar 1, 2019

CLA assistant check
All committers have signed the CLA.

@azr azr changed the title Refactor and test Add GCS Getter Mar 4, 2019
Copy link
Copy Markdown
Contributor

@azr azr left a comment

Choose a reason for hiding this comment

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

Okay, this looks good to me, nice one and thanks for opening. I tested it and it worked, I just had a question about context usage and I will try to see how we can create that bucket !

Comment thread get_gcs.go Outdated
Comment thread get_gcs_test.go Outdated
lifangmoler and others added 11 commits March 4, 2019 12:06
create google application credentials file from env vars.
quote "$GOOGLE_APPLICATION_CREDENTIALS" to avoid ambiguous redirect
output $GOOGLE_CREDENTIALS to $GOOGLE_APPLICATION_CREDENTIALS
touch "$GOOGLE_APPLICATION_CREDENTIALS" before adding contents to it
    echo %GOOGLE_CREDENTIALS% > %GOOGLE_CREDENTIALS%
create $GOOGLE_APPLICATION_CREDENTIALS file on the fly from $GOOGLE_CREDENTIALS if needed
revert file tests edits
revert file tests edits
@azr
Copy link
Copy Markdown
Contributor

azr commented Mar 5, 2019

Okay I did the testing part and tests are green, yay ! I will squash and merge to clean the commit mess.

And sorry, I forgot, could you please add doc about this getter in the README ? Otherwise this looks good to merge :)

@lifangmoler
Copy link
Copy Markdown
Contributor Author

@azr I have updated the README doc

Copy link
Copy Markdown
Contributor

@azr azr left a comment

Choose a reason for hiding this comment

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

Nice work, thanks @lifangmoler !!! 🙂

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.

3 participants