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

Fix CI interaction with moto 1.3.7 #657

Conversation

devenney
Copy link
Contributor

@devenney devenney commented Mar 3, 2019

This PR targets a feature branch - not master! See #654.

Signed-off-by: Brendan Devenney brendan@devenney.io

Resolves CI issues introduced by bumping the moto version. In short, moto currently cannot handle a lack of a default profile (or default access keys in the environment). See: getmoto/moto#1924. This appears to be fixed upstream by getmoto/moto#1952 but has not been cut for release.

Also handles the exception thrown in a more user-friendly way as this has popped previously (#310) under different circumstances.

PR Checklist

  • Wrote a good commit message & description [see guide below].
  • Commit message starts with [Resolve #issue-number].
  • Added/Updated unit tests.
  • Added/Updated integration tests (if applicable).
  • All unit tests (make test) are passing.
  • Used the same coding conventions as the rest of the project.
  • The new code passes flake8 (make lint) checks.
  • The PR relates to only one subject with a clear title.
    and description in grammatically correct, complete sentences.

Approver/Reviewer Checklist

  • Before merge squash related commits.

Other Information

Guide to writing a good commit

@devenney devenney mentioned this pull request Mar 3, 2019
@devenney
Copy link
Contributor Author

devenney commented Mar 4, 2019

@ngfgrant I've pushed changes to the Dockerfile to support Python 3.7 alongside 3.6. Seems quite a nifty solution and tox seems happy inside the container locally. If you're happy with it, I think the image update is one for you to do?

@ngfgrant
Copy link
Contributor

ngfgrant commented Mar 4, 2019

Hey @devenney thanks for the PR - circle seems to be complaining about Interpreter not found https://circleci.com/gh/cloudreach/sceptre/3509?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link :(

Copy link
Contributor

@ngfgrant ngfgrant left a comment

Choose a reason for hiding this comment

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

Generally looks good. Couple of small things.

In .circleci/config.yml could you increase the version of the docker image to 0.4? I will then push the new docker image and it should run in circle.

I'm in two minds about having the connection_manager stuff in this PR as it could resolve #621 and would probably be worthy of its own PR.

@devenney
Copy link
Contributor Author

devenney commented Mar 4, 2019

I think that's a good call. My only concern is that we won't be able to get a passing build on either PR until one has been force merged. As long as we're aware of that it's all good. Will split them out once I get some breathing room. 👍

Brendan Devenney added 2 commits March 6, 2019 17:06
* Bump CI Docker image version (to 0.4)
* Add pyenv to Docker image
* Support py27, py36, py37

Signed-off-by: Brendan Devenney <brendan.devenney@cloudreach.com>
* Works around getmoto/moto#1924 by ensuring garbage defaults are set in the environment.

Signed-off-by: Brendan Devenney <brendan.devenney@cloudreach.com>
@devenney devenney force-pushed the add-official-python-37-support branch from 0cee022 to c4a59d7 Compare March 6, 2019 17:07
@devenney
Copy link
Contributor Author

devenney commented Mar 6, 2019

@ngfgrant I've pulled the connection manager changes out of this (other than the moto 1.3.7 workaround in the tests). Should be good to build the Docker image and see if it goes green.

@ngfgrant ngfgrant merged commit 10d8921 into Sceptre:add-official-python-37-support Jun 13, 2019
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.

2 participants