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

Add deploy-bundle-arguments to deploy-bundle command #10

Conversation

denisemauldin
Copy link
Contributor

Fixes #6 by adding deploy-bundle-arguments to the deploy-bundle command.

MonoRepo to OSK [semver:major]
give get-deployment-group its own param
@denisemauldin
Copy link
Contributor Author

@gmemstr Does this work as alternate to #7 to add a separate configuration for arguments for deploy-bundle?

@denisemauldin
Copy link
Contributor Author

Also tagging @KyleTryon for visibility. I'm hoping to get this merged soon so that I don't have to go the route of the contributor to #7 and make my own copy of this orb.

@KyleTryon
Copy link
Contributor

KyleTryon commented Jan 29, 2021

Triggering tests:
edit: rebuilding alpha version and adding an empty commit to re-trigger.

@denisemauldin
Copy link
Contributor Author

@KyleTryon Is that something I need to do?

@denisemauldin
Copy link
Contributor Author

@KyleTryon I pushed a small change to see if that would help, but it seems that I can't trigger the tests?

@KyleTryon
Copy link
Contributor

Hi @denisemauldin,
We set these tests not to run on forked PRs because the pipeline relies on secrets that you will not have access to, so the pipeline would fail in either case. What I can do is pull your changes into a local branch and push it to trigger tests.

Error:

Error: Error calling command: 'deploy-bundle'
Arguments referenced without declared parameters: arguments
Error calling job: 'deploy'
Error calling command: 'deploy-bundle'
Arguments referenced without declared parameters: arguments

Locally you can run circleci orb pack src > orb.yml to generate a local packed yaml version of the orb.
Then, run circleci orb validate orb.yml for basic validation errors. We can trigger the full pipeline for the integration tests.

@denisemauldin
Copy link
Contributor Author

@KyleTryon Is the solution to this to add arguments back to deploy-bundle or to remove arguments from the deploy-bundle reference?

@KyleTryon
Copy link
Contributor

@KyleTryon Is the solution to this to add arguments back to deploy-bundle or to remove arguments from the deploy-bundle reference?

It looks like you are trying to replace the original parameter "arguments". The parameter has been renamed, but it must also be renamed where it has been utilized as well:

--query '[deploymentInfo.status]'<<# parameters.arguments >> << parameters.arguments >><</parameters.arguments >>)

<< parameters.arguments >> is still being referenced, but no longer exists as a parameter.

@denisemauldin
Copy link
Contributor Author

denisemauldin commented Feb 1, 2021

@KyleTryon Replaced arguments with get-deployment-group-arguments because deploy-bundle is running get-deployment

@KyleTryon
Copy link
Contributor

@denisemauldin could I bother you to update these linting errors: https://app.circleci.com/pipelines/github/CircleCI-Public/aws-code-deploy-orb/34/workflows/4e3c3861-4513-4697-8647-7b26f80c9270/jobs/80

I don't want to get out of sync with your fork.

Im confused it is complaining about this given, I don't think you modified this portion. Sorry about that, I wonder if a previous PR was not held to the same standard. I'll be sure once we get this PR packed up to give the whole orb a review and get rid of all of the warnings.

If you wouldnt mind just adding a blank line to the end of the deploy file to appease the hard error. The descriptions are written a little funny but it should still work (especially if you did not change them as it appears).

@denisemauldin
Copy link
Contributor Author

@KyleTryon Fixed the error and some of the warnings.

@KyleTryon
Copy link
Contributor

This issue looks to be on our end. Please allow me some time to look into this.

@denisemauldin
Copy link
Contributor Author

@KyleTryon Thanks! I appreciate your help on getting this released. :)

@KyleTryon
Copy link
Contributor

@denisemauldin Could I ask you to update the config file? There are currently some hard-coded AWs instance IDs that no longer exist, giving us our issue. I have created a new environment variable $AWS_CD_EC2_ID to replace this so that we can more easily rotate it out.

Upon revising this orb we may instead implement Terraform for our testing. Sorry for the inconvenience. If this does not work, I will attempt to create a branch and see if I can send a PR to your fork perhaps (to keep your credit for this work).

https://github.com/denisemauldin/aws-code-deploy-orb/blob/ad4a9ee7e63fe028572b5f48f34dcbe7c9c22e3e/.circleci/config.yml#L36

https://github.com/denisemauldin/aws-code-deploy-orb/blob/ad4a9ee7e63fe028572b5f48f34dcbe7c9c22e3e/.circleci/config.yml#L49

to this: aws ec2 stop-instances --instance-ids "$AWS_CD_EC2_ID"

@denisemauldin
Copy link
Contributor Author

@KyleTryon Updated the config

@denisemauldin
Copy link
Contributor Author

@KyleTryon Let me know if there's anything I can do to assist. I'm fine with you fixing this on a separate branch/PR and closing this one if it's out of date. :)

@KyleTryon
Copy link
Contributor

@KyleTryon Let me know if there's anything I can do to assist. I'm fine with you fixing this on a separate branch/PR and closing this one if it's out of date. :)

Thank you, sorry for the delay, we will get to this asap.

@KyleTryon KyleTryon self-assigned this Feb 5, 2021
@denisemauldin
Copy link
Contributor Author

@KyleTryon Thanks for your help as this is currently blocking our deploy process

@denisemauldin
Copy link
Contributor Author

@KyleTryon Hey Kyle! Do you have any updates for when this orb might be fixed? Thanks!

@KyleTryon
Copy link
Contributor

@KyleTryon Hey Kyle! Do you have any updates for when this orb might be fixed? Thanks!

Sorry again for the delays, I will take some time today on this which I will hope is enough to at least get the change out, but I will attempt to make sure this will go no longer than this week. Thank you for your patience.

KyleTryon added a commit that referenced this pull request Feb 20, 2021
* Add deploy-bundle-arguments to deploy-bundle command

Co-authored-by: Denise Mauldin <denise@noteworth.com>
@KyleTryon
Copy link
Contributor

Merged via #11. 2.0.0 released. Thank you @denisemauldin .

@KyleTryon KyleTryon closed this Feb 20, 2021
@denisemauldin
Copy link
Contributor Author

Thank you @KyleTryon !

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

2 participants