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

ModuleNotFoundError: boto3 #513

Closed
scastlara opened this issue Dec 15, 2022 · 12 comments
Closed

ModuleNotFoundError: boto3 #513

scastlara opened this issue Dec 15, 2022 · 12 comments
Labels
bug Contributions welcome Help on this would be much appreciated good first issue

Comments

@scastlara
Copy link
Contributor

Hey! After installing django-sql-explorer==3.0, I get this error:

[2022-12-15T14:06:49.875Z] #16 2.041   File "/app/venv/lib/python3.10/site-packages/explorer/utils.py", line 9, in <module>
[2022-12-15T14:06:49.875Z] #16 2.041     import boto3
[2022-12-15T14:06:49.875Z] #16 2.041 ModuleNotFoundError: No module named 'boto3'

I see boto3 is actually imported directly here https://github.com/groveco/django-sql-explorer/blob/master/explorer/utils.py#L9, but when installing this app boto3 is not installed as a dependency.

Am I missing something?

Thanks!

@marksweb
Copy link
Collaborator

@scastlara thanks for raising.

I think I need to add another section of tests to run without the optional requirements. Then the package extras need an entry point for boto integration.

I've not caught this because everything I do is on aws so boto is a default.

My Internet is down at the moment so I'll look at it when that's resolved. Unless someone can raise a PR in the meantime.

@marksweb marksweb added bug Contributions welcome Help on this would be much appreciated good first issue labels Dec 15, 2022
@marksweb
Copy link
Collaborator

Ok I managed to make a start on this but the test run without the extra packages causes test failures so it needs more work than I'd hoped.

🤞 For a patch today.

@lawson89
Copy link
Contributor

@marksweb should we conditionally import boto only if the s3 bucket is set?

@marksweb
Copy link
Collaborator

@lawson89 Thats sort of what I've done.

I've moved the boto import to the get_s3_bucket function which will only get called if you're running tasks. The trouble now is that the test views expect to be able to use snapshots and they're now failing so I'm trying to get an understanding of what the views are doing that the tests are expecting (not my code so needs a bit more thought than usual)

You can see progress here; master...fix/optional-packages

@lawson89
Copy link
Contributor

Ok moving that import makes sense - let me know if you need some help and I'm glad to take a look at the test cases

@marksweb
Copy link
Collaborator

@lawson89 If you can that'd be great. Can you branch from that fix/optional-packages branch? I'm going to be unavailable for a few hours & then I've just been asked to look at something else.

Then stop tox from installing the optional packages & have it run on 1 version of python & django - that should be the fastest way to run the test suite.

There's currently 2 failures and 7 errors from the suite. The initial focus of changes is to get things running without the optional extra packages, so xlswriter, celery & boto are the obvious ones from the test issues.

I've got the github actions to then run the tests from the basic requirements, then the optional requirements separately.

@lawson89
Copy link
Contributor

Ok looking into it now

@lawson89
Copy link
Contributor

Ok I have the tests failing locally so will fix those up

I think the tests are not coded to handle optional dependencies gracefully is the main issue

@marksweb
Copy link
Collaborator

@lawson89 yeah it's all setup with the expectation that packages are available because the tests have always installed everything.

It'd make sense if the views which depend on extra packages threw 404s when the packages are missing or settings disabled. And furthermore, if settings enabled and packages not installed it should be throwing the improperly configured exception.

@lawson89
Copy link
Contributor

Ok I added skips like: @unittest.skipIf(not app_settings.ENABLE_TASKS, 'tasks not enabled')

I have all the tests either passing or being skipped if functionality not configured (or module not available)
Python3.8, Django 3.2. I'll put in a PR so you can review and see if you agree or if you have suggestions

@marksweb
Copy link
Collaborator

@lawson89 sounds perfect. Thanks 👍

lawson89 added a commit to lawson89/django-sql-explorer that referenced this issue Dec 15, 2022
marksweb pushed a commit that referenced this issue Dec 15, 2022
marksweb added a commit that referenced this issue Dec 15, 2022
* Ensure there is a workflow run without optional packages

* Catch import error on celery

* Move boto3 to local import if the s3 upload is being used.

* Added workflow concurrency

* Add ENABLE_TASKS as env variable for tests

* Move factory boy into base test requirements

* Skip unit tests as appropriate if functionality not configured (tasks) or loadable (xls-writer) (#514)

Issue #513

Co-authored-by: Richard Lawson <lawson89@users.noreply.github.com>
@marksweb
Copy link
Collaborator

@scastlara I've just released 3.0.1 which should allow you to run the app without boto installed.

Thank you for your help again @lawson89!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Contributions welcome Help on this would be much appreciated good first issue
Projects
None yet
Development

No branches or pull requests

3 participants