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

added unique token to deployment logical id #1761

Merged
merged 3 commits into from Aug 4, 2016
Merged

Conversation

eahefnawy
Copy link
Member

Adds a unique timestamp token to the APIG deployment logical id to prompt AWS CF to restart deployment.

@eahefnawy eahefnawy added this to the v1.0.0-beta.2 milestone Aug 4, 2016
@eahefnawy eahefnawy self-assigned this Aug 4, 2016
@flomotlik
Copy link
Contributor

LGTM

@eahefnawy eahefnawy merged commit 67f871e into master Aug 4, 2016
@eahefnawy eahefnawy deleted the deployment-id branch August 4, 2016 19:52
@wedgybo
Copy link
Contributor

wedgybo commented Aug 4, 2016

I'm looking over the PR on a phone but I'm pretty sure this won't cover removed functions

@flomotlik
Copy link
Contributor

@wedgybo yup it looks like its unreliable for removing methods from a deployment. It reliably adds them now due to the dependsOn, but the takes old methods with it most of the time as old methods are still available in the resources while the new deployment is happening. Just as you described in the issue. I'll bring this up with AWS, because this is really seems like a bug in the Cloudformation implementation now.

@wedgybo
Copy link
Contributor

wedgybo commented Aug 5, 2016

Yea the problem is it works on a diff so you need to deploy after clean up.

I THINK the solution is to create the stage without relying on the deployment to do it for you, if you want to do it all in one swoop. So it would be worth asking about that approach?

@flomotlik
Copy link
Contributor

@wedgybo yup reached out, let see what comes back. but at least we can deploy in general now.

@kkozmic-seek
Copy link

kkozmic-seek commented Nov 28, 2016

What problem does this solve (the unique Deployment logical ID specifically)? Having multiple Deployments with unique name kills ability to specify StageDescription as now subsequent deployments fail with "StageDescription cannot be specified when stage referenced by StageName already exists"

@pmuens
Copy link
Contributor

pmuens commented Nov 28, 2016

Hey @kkozmic-seek this is a kind of workaround so that API Gateway knows that a new deployment should be done / something in the config was updated.

API Gateway won't pick up the updates you've specified if this is missing.

@kkozmic-seek
Copy link

hey @pmuens thanks for the response.

So let me see if I understand correctly what you're saying.

If we have hardcoded Deployment logical ID that we reuse every time API Gateway will not deploy subsequent changes to the API? That feels... counterintuitive.

Also, if that's the case, why was it only changed late in the process for Serverless (this PR was only merged few months ago). Wouldn't that issue be visible much sooner than that?

@pmuens
Copy link
Contributor

pmuens commented Nov 29, 2016

@kkozmic-seek correct.
The re-deployment only failed sometimes as far as I remember. Don't know which edge cases were affected but maybe @eahefnawy knows more.

The API Gateway team is aware of this and they're looking into it to fix it.

@kkozmic-seek
Copy link

Ok, I opened a ticket with AWS about this as well, and I'm looking at some workarounds for my underlying issue (API Gateway caching support) which looks like may be solved by pulling Stage into it's own explicit resource in CFN and pointing from Stage to Deployment.

I have some options to try so hopefully one of them does work reliably

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

5 participants