Closed alekhrycaiko closed 4 years ago
@lokst What's the best way of going about testing any new changes for contributing to this repository? I haven't seen anything about how to go about this in the repo (unless I've missed it).
@alekhrycaiko You can edit .circleci/config.yml to make sure the new behavior is covered by the tests. I'll then run the build for you
Thanks @lokst I've added a case that'll cover the new functionality.
On a related note, I realized the test suite is covering the functionality I want to include - so I've gone ahead and used that approach here instead - it seems like a better one.
@alekhrycaiko Thank you for your contributions! Could you review my feedback?
Not a problem, and thanks for the feedback. I've provided the changes as requested.
I had one follow up concern, but, I'm unsure if we should deal with this here, or another PR. I've been running changes similar to this in my own pipeline and have noticed CircleCI's default timeout of 10minutes creates some friction with the wait
for Code deploy. For example, if your deployment takes 10-60mins to deploy, CircleCI will fail after 10minutes despite the wait being enabled.
I've installed an extension to my timeout via no_output_timeout
but I'm uncertain if this is a valid key for the job we're modifying. If it is not a valid key, perhaps we need an extra parameter to control this timeout?
Let me know what you think. Happy to add this in here if it's desirable (or in a subsequent PR).
@alekhrycaiko I think adding something like a verification-timeout
parameter (where it's specified int the description that it's only applicable to ECS services of the Blue/Green Deployment type) will be a great addition to this PR! Here is a reference from another orb that does something similar, if that helps: https://github.com/CircleCI-Public/aws-eks-orb/blob/1a6c5cedd3b5679fad032161b5280c3cdf91b6d7/src/commands/create-cluster.yml#L369
I've done some follow up @lokst this should be reviewable again. Let me know if there's any concerns with the approach, and thanks again for the eyes. :+1:
@alekhrycaiko Thank you, I'll review the changes as soon as possible!
@alekhrycaiko Apologies for not having got back to you yet, this is still on our radar of things to do!
@alekhrycaiko Taking a look at this today, I found some minor syntax issues and will push a commit to get the build working and integration tests running 🙂
Great! Thank you :)
On Aug 6, 2020, at 10:30 PM, Stella Lok notifications@github.com wrote:
 @alekhrycaiko Taking a look at this today, I found some minor syntax issues and will push a commit to get the build working and integration tests running 🙂
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.
@alekhrycaiko Thanks for your patience and contribution, your changes are now available in version 1.3.0 of the orb!
Checklist
Motivation, issues
When using the orb in it's current state, deployment statuses of the blue-green deployment aren't reported. As a result I get no feedback as to when my new revision's deployment has failed, or succeeded in placing tasks.
A similar issue has been reported by another user, and can be seen in the issue below.
https://github.com/CircleCI-Public/aws-ecs-orb/issues/93
Description
Adds functionality for the CircleCI parameter
verify-revision-is-deployed
for Code_deploy controllers.To my understanding the command above doesn't exist on the AWS CLI, but, is being emulated by waiting for the deployment.