Skip to content

Conversation

@nathanagood
Copy link
Contributor

Proposed changes

Changes to resolve the following issues:

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (changes to code, which do not change application behavior)

Checklist

  • I have filled out this PR template
  • I have read the CONTRIBUTING doc
  • I have added automated tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (README.md, inline comments, etc.)
  • I have updated the CHANGELOG.md under a ## next release, with a short summary of my changes

Relevant Links

Further comments


args := argsFromOverrides(overrides)
s.Util.Terraformer.Apply(args)
s.Util.Terraformer.Apply(ctx, []string{})
Copy link
Contributor

@joshmarsh joshmarsh Jan 10, 2020

Choose a reason for hiding this comment

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

Maybe we should remove or change the above log statement. The output makes it look like it was already applied before asking if you want to apply dce resources

~ ./dce-cli system deploy
Creating DCE infrastructure
Initializing terraform working directory
Applying DCE infrastructure
Are you sure you would like to create DCE resources?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch. I'll clean these up to show a better flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in latest push. The output now looks like this:

Initializing
Creating DCE infrastructure
Are you sure you would like to create DCE resources? (must type "yes" if yes)	yes
Retrieving artifacts location
Artifacts bucket =  574035908831-dce-artifacts-dce-bmddqqcb
Deploying code assets to DCE infrastructure
Downloading DCE code assets
.
.
.
.
.
.
.
.
.
.
.
Updating AWS Lambda functions...
Finished updating AWS Lambda functions.

@eschwartz
Copy link
Contributor

@nathanagood -- do you think we could do an in person review of this PR, given its size?

@nathanagood
Copy link
Contributor Author

@nathanagood -- do you think we could do an in person review of this PR, given its size?

@eschwartz - If by "in-person" you mean over vid, sure. I'd hate to wait until we are physically in-person.

@joshmarsh joshmarsh mentioned this pull request Jan 10, 2020
9 tasks
@nathanagood nathanagood merged commit a6c1740 into Optum:master Jan 10, 2020
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.

3 participants