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

add tox and switch to GithubAction #97

Closed
wants to merge 3 commits into from

Conversation

vincentsarago
Copy link
Contributor

πŸ‘‹ This pr does:

  • switch to GH actions πŸ€·β€β™‚οΈ
  • add TOX for testing multiple python versions πŸ€·β€β™‚οΈ

To be discussed

  • pre-commit integration?
  • pypi publish from CI?
  • mkdoc / gh-page?

with:
aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }}
aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
aws-region: us-east-1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess you need AWS credential to be able to use S3 file πŸ€·β€β™‚οΈ

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah should really use moto so we don't need to talk to S3 at all for tests

name: ${{ matrix.platform }}-${{ matrix.tox-env }}
fail_ci_if_error: false

# publish:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TDB

tox.ini Outdated
deps=
numpy
passenv=
AWS_*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can remove this if S3 credential is not needed

@vincentsarago
Copy link
Contributor Author

vincentsarago commented Dec 17, 2020

when trying to run the test within tox I get

ImportError while loading conftest '/Users/vincentsarago/Workspace/vincentsarago/aiocogeo/tests/conftest.py'.
tests/conftest.py:9: in <module>
    from aiocogeo import config
aiocogeo/__init__.py:1: in <module>
    from .cog import COGReader, CompositeReader
aiocogeo/cog.py:16: in <module>
    from .filesystems import Filesystem
aiocogeo/filesystems.py:19: in <module>
    import aioboto3
.tox/py37/lib/python3.7/site-packages/aioboto3/__init__.py:5: in <module>
    from aioboto3.session import Session
.tox/py37/lib/python3.7/site-packages/aioboto3/session.py:9: in <module>
    import aiobotocore.session
.tox/py37/lib/python3.7/site-packages/aiobotocore/__init__.py:1: in <module>
    from .session import get_session, AioSession
.tox/py37/lib/python3.7/site-packages/aiobotocore/session.py:11: in <module>
    from .credentials import create_credential_resolver, AioCredentials
.tox/py37/lib/python3.7/site-packages/aiobotocore/credentials.py:15: in <module>
    from botocore.credentials import EnvProvider, Credentials, RefreshableCredentials, \
E   ImportError: cannot import name 'SSOProvider' from 'botocore.credentials' (/Users/vincentsarago/Workspace/vincentsarago/aiocogeo/.tox/py37/lib/python3.7/site-packages/botocore/credentials.py)
ERROR: InvocationError for command /Users/vincentsarago/Workspace/vincentsarago/aiocogeo/.tox/py37/bin/python -m pytest --cov aiocogeo --cov-report xml --cov-report term-missing (exited with code 4)

😬

ref: aio-libs/aiobotocore#824

@@ -14,8 +14,6 @@
"pytest-asyncio<0.11.0",
"pytest-cov",
"shapely",
"botocore==1.15.32",
"boto3==1.12.32",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

aioboto3, depends on aiobotocore which depends on specific boto3 and botocore versions. It's easier to let hem decide the version they need ;-)

tox.ini Outdated
@@ -7,8 +7,6 @@ commands=
python -m pytest --cov aiocogeo --cov-report xml --cov-report term-missing
deps=
numpy
passenv=
AWS_*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is not needed in fact!

@geospatial-jeff
Copy link
Owner

I pulled down your changes, ran tox and it worked πŸŽ‰

@geospatial-jeff geospatial-jeff marked this pull request as ready for review December 17, 2020 23:15
@kylebarron
Copy link

You might be able to merge this onto a non-master branch, and then CI might run?

@geospatial-jeff
Copy link
Owner

Finished in #99 !

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.

3 participants