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.

Sign up for GitHub

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

Jump to bottom

POC: Use AWS S3 buckets #22

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

POC: Use AWS S3 buckets #22

wants to merge 3 commits into from

Conversation

pllim
Copy link
Contributor

@pllim pllim commented Mar 28, 2019 •
edited

@nmiles2718
Copy link
Collaborator

nmiles2718 commented Mar 29, 2019 •
edited

Too much unnecessary cruft here. All that needs to be done is for the user to execute Observations.enable_cloud_dataset() in a cell proceeding the call to download found observations. No need for all the intermediate steps. Note that if we want to list this option for the user, we will need to do some more work to ensure that they have boto3 properly configured.

Screen Shot 2019-03-29 at 10 52 06 AM

@nmiles2718 nmiles2718 self-assigned this Mar 29, 2019
@nmiles2718 nmiles2718 added the enhancement New feature or request label Mar 29, 2019
@nmiles2718 nmiles2718 self-requested a review March 29, 2019 15:10
nmiles2718
nmiles2718 requested changes Mar 29, 2019
View reviewed changes
Copy link
Collaborator

@nmiles2718 nmiles2718 left a comment •
edited

Choose a reason for hiding this comment

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

Remove all steps that involve converting the URIs to S3 URIs and instead insert cell at the very beginning with Observations.enable_cloud_dataset()`. (See initial comment)

@pllim
Copy link
Contributor Author

pllim commented Mar 29, 2019

@nmiles2718 , thank you very much for the review, which definitely cleaned things up. I think I have addressed your comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nmiles2718 nmiles2718 nmiles2718 requested changes

Assignees

@nmiles2718 nmiles2718

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@pllim @nmiles2718