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

Import Botocore from a more specific location #100

Merged
merged 1 commit into from Feb 1, 2018
Merged

Import Botocore from a more specific location #100

merged 1 commit into from Feb 1, 2018

Conversation

ghost
Copy link

@ghost ghost commented Feb 1, 2018

Importing Botocore directly from tornado_botocore.base allows us to bypass the vague error message given by the library and see the actual import failure.
Fixes #99

Importing Botocore directly from `tornado_botocore.base` allows us to bypass the vague error message given by the library and see the actual import failure.
Fixes #99
@Bladrak Bladrak self-requested a review February 1, 2018 08:45
@ghost
Copy link
Author

ghost commented Feb 1, 2018

As far as I can tell this change doesn't change the supported versions. The Botocore class has always resided in tornado_botocore.base and the message which masks the actual import error was added in tornado-botocore v0.1.0.

In the end this change only clears up the error message. In my case and in the source for the issue in #99, the root cause for incompatibility was in missing dependencies down the stream.

@Bladrak
Copy link

Bladrak commented Feb 1, 2018

All right then, thanks for contributing!

@Bladrak Bladrak merged commit b6b6c15 into thumbor-community:master Feb 1, 2018
@Bladrak
Copy link

Bladrak commented Feb 1, 2018

@sami9gag seems the tests are failing following your change, do you mind taking a look? (I'll try to see why circle didn't test your PR)

@Bladrak
Copy link

Bladrak commented Feb 1, 2018

My bad @sami9gag your change is not the cause of the tests failing

@ghost
Copy link
Author

ghost commented Feb 1, 2018

I noticed that CircleCI didn't test my change, which seemed a bit odd.

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.

1 participant