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

Fixing Minio newest package version incompability #847

Merged
merged 8 commits into from
Dec 10, 2020
Merged

Fixing Minio newest package version incompability #847

merged 8 commits into from
Dec 10, 2020

Conversation

Synicix
Copy link
Contributor

@Synicix Synicix commented Dec 10, 2020 •
edited
Loading

For now I changed to except all errors to make it work. I need to dig more into how minio does error handling to shrink the error scope to be more specific.

@Synicix Synicix changed the title Update exception to more general form. Need to find out exactly what … Fixing Minio newest package version incompability Dec 10, 2020
@Synicix Synicix mentioned this pull request Dec 10, 2020
Merged
@Synicix Synicix marked this pull request as ready for review December 10, 2020 06:37
@guzman-raphael
Copy link
Collaborator

guzman-raphael commented Dec 10, 2020 •
edited
Loading

@Synicix Looks like from here you can check minio.error.S3Error.code. It appears that the code translation is done based on this mapping. Also, I would also do a backward compatibility check to see what breaks with these changes but setting minio<7.0.0 in requirements.txt. After discussing with others, we have agreed that it is reasonable since it is a major update for minio to require in our dependencies minio>=7.0.0 with this update. Please add this in as well.

guzman-raphael
guzman-raphael requested changes Dec 10, 2020
View reviewed changes
datajoint/s3.py Outdated Show resolved Hide resolved
guzman-raphael
guzman-raphael approved these changes Dec 10, 2020
View reviewed changes
Copy link
Collaborator

@guzman-raphael guzman-raphael 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! 🚀

@guzman-raphael guzman-raphael merged commit 91b70a6 into datajoint:master Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dimitri-yatsenko dimitri-yatsenko dimitri-yatsenko approved these changes

@guzman-raphael guzman-raphael guzman-raphael 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.
3 participants
@Synicix @guzman-raphael @dimitri-yatsenko