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 testing pipeline #580

Merged
merged 5 commits into from
Mar 27, 2023
Merged

Fix testing pipeline #580

merged 5 commits into from
Mar 27, 2023

Conversation

hughcapet
Copy link
Collaborator

@hughcapet hughcapet commented Feb 28, 2023

  • CDP overlays -> runtime
  • Remove top-level region option
  • Define a separate Command class for flake8 in setup.py (flake8 setuptools/setup.py support is now deprecated)
  • Fix a flake8 error

@hughcapet hughcapet force-pushed the ci/fix-pipeline branch 6 times, most recently from f222900 to 16f4d4f Compare March 2, 2023 07:34
The current implementation uses value set via the envvar AWS_DEFAULT_REGION
for sub-commands even if an explicit flag value is provided.

This commit also integrates get_region() into the 'region' option's
callback, so that senza first ensures a non-empty region value (gets
region from "~/.aws/config" if nothing is provided) and then validates
it against the available regions list.
As flake8 support for setup.py has been removed
@hughcapet hughcapet marked this pull request as ready for review March 2, 2023 08:11
@Jan-M
Copy link
Contributor

Jan-M commented Mar 15, 2023

👍

@jmcs
Copy link
Member

jmcs commented Mar 15, 2023

LGTM but I've a question. Why are you removing the top-level region option?

@hughcapet
Copy link
Collaborator Author

@jmcs

sorry, should have posted the message from one of the commits. This is because this was causing the following misbehavior (revealed in the test_cli.py unit tests):

The current implementation uses value set via the envvar AWS_DEFAULT_REGION
for sub-commands even if an explicit flag value is provided.

And I actually don't fully understand the whole idea of having the top-level region option.

@hughcapet
Copy link
Collaborator Author

👍

@hughcapet hughcapet merged commit 6c53449 into master Mar 27, 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.

3 participants