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

Patch wine & iris dataset requests #40

Merged
merged 2 commits into from
Aug 11, 2023
Merged

Patch wine & iris dataset requests #40

merged 2 commits into from
Aug 11, 2023

Conversation

t-rutten
Copy link
Contributor

Resolves #39

@t-rutten t-rutten requested a review from wojtekmach August 10, 2023 23:38
@wojtekmach
Copy link
Contributor

If these datasets never change I would consider calculating the sha of response body once, hardcoding in the source, and check against when we download. LGTM otherwise.

@t-rutten
Copy link
Contributor Author

Thanks for the suggestion @wojtekmach. These datasets indeed do not change--I've added a comparison against their hashes.

wojtekmach
wojtekmach approved these changes Aug 11, 2023
View reviewed changes
Copy link
Contributor

@wojtekmach wojtekmach left a comment

Choose a reason for hiding this comment

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

Awesome ship it!

@t-rutten t-rutten merged commit ace9be1 into master Aug 11, 2023
@t-rutten t-rutten deleted the patch-iris branch August 11, 2023 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wojtekmach wojtekmach wojtekmach approved these changes

@josevalim josevalim josevalim approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Iris and Wine dataset download fails with Bad Certificate
3 participants
@t-rutten @wojtekmach @josevalim