Navigation Menu

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

Introduce completely separate chalice stages #264

Merged
merged 3 commits into from Mar 29, 2017

Conversation

jamesls
Copy link
Member

@jamesls jamesls commented Mar 28, 2017

This introduces a change to how stages work in chalice.
Previously a "stage" in chalice corresponded to an API gateway
stage. While this could theoretically work for lambda functions
as well (via aliasing and function versions) it would not necessarily
scale with other AWS resources. Now, chalice "stages" are
entirely new sets of resources. For example, if I run:

$ chalice deploy dev  # Note 'dev' can be omitted, it's the default
$ chalice deploy prod

I will have two completely separate sets of API gateway rest APIs,
Lambda functions, and IAM roles.
To help track this, a ".chalice/deployed.json" is written on deploys
that contains the deployed values per-stage.

There's still some additional work remaining, primarily against
upgrading from old versions to this new version. We should be warning
or erroring out in that scenario, but that will be addressed as a
separate commit.

Additionally, the "chalice url/logs" command can be updated to use
the ".chalice/deployed.json" file as well, but that will be added as
a separate commit.

Copy link
Member

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Looks good. For the future PR, I think warning with a prompt when upgrading and using chalice deploy with a stage is the best approach. Just had a couple small comments. I am not sure how the error case would look and the user workflow from that would look.

try:
self._client('apigateway').get_rest_api(restApiId=rest_api_id)
return True
except botocore.exceptions.ClientError as e:
Copy link
Member

Choose a reason for hiding this comment

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

I think you can use modeled exceptions for this one if you want. NotFoundException is modeled in api gateway's service model.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I'll get these updated, there's a few places I'm catching ClientError that can be updated. I'll just need to bump the min botocore version to 1.5.0.



class DeployedResources(object):
def __init__(self, backend, api_handler_arn,
Copy link
Member

Choose a reason for hiding this comment

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

That's a lot of arguments. Wonder if this can be simplified but seems to be in all practicality is a namedtuple. So I don't feel too strongly about needing to change it.

@codecov-io
Copy link

Codecov Report

Merging #264 into package will decrease coverage by 0.57%.
The diff coverage is 75.45%.

Impacted file tree graph

@@             Coverage Diff             @@
##           package     #264      +/-   ##
===========================================
- Coverage    82.91%   82.33%   -0.58%     
===========================================
  Files           17       17              
  Lines         1539     1585      +46     
  Branches       193      198       +5     
===========================================
+ Hits          1276     1305      +29     
- Misses         212      227      +15     
- Partials        51       53       +2
Impacted Files Coverage Δ
chalice/cli/__init__.py 55.23% <20%> (-0.72%) ⬇️
chalice/awsclient.py 77.85% <22.22%> (-3.71%) ⬇️
chalice/utils.py 75.75% <70%> (-3.41%) ⬇️
chalice/config.py 89.39% <72%> (-10.61%) ⬇️
chalice/deploy/deployer.py 73.43% <90.16%> (+2%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a5ca14...93aa095. Read the comment docs.

This introduces a change to how stages work in chalice.
Previously a "stage" in chalice corresponded to an API gateway
stage.  While this could theoretically work for lambda functions
as well (via aliasing and function versions) it would not necessarily
scale with other AWS resources.  Now, chalice "stages" are
entirely new sets of resources.  For example, if I run:

$ chalice deploy dev  # Note 'dev' can be omitted, it's the default
$ chalice deploy prod

I will have two completely separate sets of API gateway rest APIs,
Lambda functions, and IAM roles.
To help track this, a ".chalice/deployed.json" is written on deploys
that contains the deployed values per-stage.

There's still some additional work remaining, primarily against
upgrading from old versions to this new version.  We should be warning
or erroring out in that scenario, but that will be addressed as a
separate commit.

Additionally, the "chalice url/logs" command can be updated to use
the ".chalice/deployed.json" file as well, but that will be added as
a separate commit.
This was added in v1.5.0 of botocore so I've bumped the min
version of botocore accordingly.
Copy link
Member

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Thanks. Looks good. 🚢

@jamesls jamesls merged commit 06b3561 into aws:package Mar 29, 2017
@recursify
Copy link

@jamesls - thanks for doing this! Chalice seemed promising, but was a non-starter unless it had full dev/prod/test environments. I'll check out the package branch and give it a whirl.

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

4 participants