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

replace circleci docker image #1394

Closed
wants to merge 8 commits into from

Conversation

zaro0508
Copy link
Contributor

@zaro0508 zaro0508 commented Nov 28, 2023

Before setting up Poetry for this project we had to create a custom docker container[1] with all the virtualenvs for tests. Poetry is now handling the virtualenv for tests so we no longer need to use the custom docker container for running tests, we can just use the stock circleci container.

[1] https://github.com/Sceptre/sceptre-circleci

Poetry is handling the virtualenv for tests so we no longer need
to use a custom docker container for running tests.
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@iainelder
Copy link
Contributor

I'm a bit lost now among so many PRs.

Does this change make the sceptre-circleci repo and the PR we just worked on obsolete?

@zaro0508
Copy link
Contributor Author

Does this change make the sceptre-circleci repo and the PR we just worked on obsolete?

Yes, that is correct @iainelder

@iainelder
Copy link
Contributor

iainelder commented Dec 1, 2023

It looks like the stock CircleCI container doesn't make available all the Python feature versions we need.

Maybe we do still need a custom image after all.

Use this command to check the reported version of each target Python feature version.

docker run -it cimg/python:3.12 bash -c 'for py in python3.{8,9,10,11,12}; do "$py" --version; done'

Python 3.10 and 3.12 report back. Python 3.8, 3.9, and 3.11 are missing.

bash: line 1: python3.8: command not found
bash: line 1: python3.9: command not found
Python 3.10.6
bash: line 1: python3.11: command not found
Python 3.12.0

I discovered it running the unit tests as the CI pipeline would.

docker run -it cimg/python:3.12 bash -c '
git clone https://github.com/Sceptre/sceptre .
git fetch origin pull/1394/head:pr
git switch pr
poetry install --all-extras -v
poetry run tox
echo "Tox exit code: $?"
'

Tox passes py310. It skips py38, py39, and py311 because it didn't find the interpreter.

SKIPPED:  py38: InterpreterNotFound: python3.8
SKIPPED:  py39: InterpreterNotFound: python3.9
  py310: commands succeeded
SKIPPED:  py311: InterpreterNotFound: python3.11
  congratulations :)
Tox exit code: 0

The worst part is that Tox exits with code 0.

The 0 exit code means CircleCI run 11003 for the build-and-unit-test workflow reported success.

I would prefer that Tox fails when it can't find the target interpreter.

I can make that happen by deleting the skip_missing_interpreter line from its config.

skip_missing_interpreters = true

docker run -it cimg/python:3.12 bash -c '
git clone https://github.com/Sceptre/sceptre .
git fetch origin pull/1394/head:pr
git switch pr
sed -i "/skip_missing_interpreters/d" tox.ini
poetry install --all-extras -v
poetry run tox
echo "Tox exit code: $?"
'

Tox still runs all the tests it can, but now it fails because of missing interpreters.

ERROR:  py38: InterpreterNotFound: python3.8
ERROR:  py39: InterpreterNotFound: python3.9
  py310: commands succeeded
ERROR:  py311: InterpreterNotFound: python3.11
Tox exit code: 1

@zaro0508
Copy link
Contributor Author

zaro0508 commented Dec 1, 2023

i didn’t realize that but you are correct @iainelder. Unit test fails after setting skip_missing_interpreters = false in this PR using cimg/python:3.12. It passes when setting skip_missing_interpreters = false in PR #1393 using sceptreorg/sceptre-circleci:2.1.0. i guess we do still need the sceptre-circleci docker images, will close this PR and reopen #1393.

@zaro0508 zaro0508 closed this Dec 1, 2023
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.

None yet

2 participants