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

Update benchmarking user guide for reading private datasets #427

Merged
merged 1 commit into from
May 14, 2021

Conversation

katxiao
Copy link
Contributor

@katxiao katxiao commented May 12, 2021

Update benchmarking user guide for reading private datasets

@katxiao katxiao requested a review from csala May 12, 2021 04:31
@katxiao katxiao marked this pull request as ready for review May 12, 2021 04:31
Copy link
Contributor

@csala csala left a comment

Choose a reason for hiding this comment

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

This looks good. I would just add a comment about being able to access private buckets without the need to pass credentials explicitly if they have been configured system-wide.

Running on Private Datasets
~~~~~~~~

If we want to run sdgym on datasets in a private s3 bucket, we can pass in
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rephrase this a bit to mention something like "in a private S3 bucket for which we do not have system-wide credentials configured", and link the corresponding part of the sentence to the AWS documentation that explains how to configure .aws/credentials or set environment variables.

@katxiao katxiao changed the title Update benchmarking user guide for reading private datasets Update benchmarking user guide for reading private datasets (PR 1/3) May 12, 2021
@katxiao katxiao changed the title Update benchmarking user guide for reading private datasets (PR 1/3) Update benchmarking user guide for reading private datasets May 12, 2021
Copy link
Contributor

@csala csala left a comment

Choose a reason for hiding this comment

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

Looks good @katxiao ! Let's leave this in standby while the corresponding changes are finished and merged in SDGym, and then we are ready to merge this.

@katxiao katxiao merged commit d4302de into master May 14, 2021
@katxiao katxiao deleted the update-benchmarking-doc branch May 14, 2021 18:44
@katxiao katxiao added this to the 0.10.0 milestone May 21, 2021
@csala csala added the documentation Docs, user guides or API label May 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Docs, user guides or API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants