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

Change the default api gateway stage name to v1 #431

Merged
merged 3 commits into from
Jul 28, 2017

Conversation

jamesls
Copy link
Member

@jamesls jamesls commented Jul 26, 2017

There's some confusion about chalice stages and api gateway stages
and to make this more clear, I propose changing the default value
for api gateway stages to v1.

This is a backwards compatible change because this value is pulled
from the .chalice/config.json. This is just changing what the
default value is that's written to the config file as part of the
new-project command.

Also updated the docs to use v1 to avoid confusion.

@jamesls jamesls requested review from stealthycoin and kyleknap July 26, 2017 21:17
@codecov-io
Copy link

codecov-io commented Jul 26, 2017

Codecov Report

Merging #431 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #431      +/-   ##
=========================================
+ Coverage    93.6%   93.6%   +<.01%     
=========================================
  Files          18      18              
  Lines        2861    2863       +2     
  Branches      375     375              
=========================================
+ Hits         2678    2680       +2     
  Misses        132     132              
  Partials       51      51
Impacted Files Coverage Δ
chalice/cli/__init__.py 81.86% <100%> (+0.1%) ⬆️
chalice/constants.py 100% <100%> (ø) ⬆️

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 2353fc0...263cfbb. Read the comment docs.

Copy link
Contributor

@stealthycoin stealthycoin 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.

@jamesls
Copy link
Member Author

jamesls commented Jul 26, 2017

Capturing offline discussion: Should we change the default to api instead of v1. This seems to be closer to how chalice thinks of API gateway stages, and we don't support any kind of stage configuration in API gateway for the time being. It would make this concept less confusing.

Thoughts?

@kyleknap
Copy link
Contributor

I am in favor of something like api as opposed to v1 for the reason that if a user actually wanted to allow versioning of my api in the future, the v1 is misleading as it would not be hard to assume it would be easy to add a v2 to my API. However, it is not as chalice does not support multiple API Gateway stages per chalice stage. For example, if I changed my chalice APIG stage name to v2, the v1 would go away which would completely defeat the purpose of versioning.

Making it named api helps that if I was thinking about versioning, I would be more likely to embed v1 into my route definition decorator, which is what you would have to do currently in chalice if you want your API version embedded in the url.

@jamesls
Copy link
Member Author

jamesls commented Jul 27, 2017

Ok I'll update the PR to use api instead and I'll try it out for a bit.

@jamesls jamesls force-pushed the change-default-stage branch from 35e4809 to 241ea14 Compare July 27, 2017 17:40
@jamesls
Copy link
Member Author

jamesls commented Jul 27, 2017

Pushed an update that changes the stage name to api.

Copy link
Contributor

@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.

I like the use of api out of all the options we have considered. 🚢

jamesls added 3 commits July 28, 2017 15:16
There's some confusion about chalice stages and api gateway stages
and to make this more clear, I propose changing the default value
for api gateway stages to v1.

This is a backwards compatible change because this value is pulled
from the .chalice/config.json.  This is just changing what the
default value is that's written to the config file as part of the
new-project command.
Based on PR discussion.
@jamesls jamesls force-pushed the change-default-stage branch from 241ea14 to 263cfbb Compare July 28, 2017 22:18
@jamesls jamesls merged commit 263cfbb into aws:master Jul 28, 2017
@edwinevans
Copy link

I'm confused by this. I used to have a chalice project with some common code that supported 2 different apis (and code would check where it is being called from):

foo.mycompany.com/dev
bar.mycompany.com/dev

I could deploy to different stages using chalice deploy dev or chalice deploy prod etc and it would create endpoints like you expect. Before deploying I had a script that would update chalice config.json to change the app_name. But now I only get foo.mycompany.com/api. I tried changing deployed.json which could allow different path but it seems to be ignoring app name and I don't think this is intended way to do it, but --stage doesn't seem to be doing anything. Any suggestion?

@edwinevans
Copy link

Is there any way in latest version of chalice to handle my use case where I have 2 different APIs that need multiple stages generated from the same source?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants