Skip to content

Conversation

@katiewest820
Copy link
Contributor

Per meeting on Tuesday:

  • Add additional logging for integrations command to notify customer when managed secret isn't set up for the account id they're using.
  • Add an error during layers command when NR Account id doesn't match one stored in secret.
  • In warning and error messages point users to updated docs that outline how to manually set up managed secret.
  • Add key/val of account id associated with stored license key in secret manager.
    https://newrelic.atlassian.net/browse/LAMBDA-1197

@codecov
Copy link

codecov bot commented May 6, 2021

Codecov Report

Merging #171 (7109ee3) into master (d538c7b) will increase coverage by 0.13%.
The diff coverage is 79.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #171      +/-   ##
==========================================
+ Coverage   76.96%   77.09%   +0.13%     
==========================================
  Files          16       16              
  Lines        1211     1227      +16     
==========================================
+ Hits          932      946      +14     
- Misses        279      281       +2     
Impacted Files Coverage Δ
newrelic_lambda_cli/integrations.py 65.86% <74.07%> (+0.44%) ⬆️
newrelic_lambda_cli/layers.py 87.65% <100.00%> (+0.77%) ⬆️
newrelic_lambda_cli/utils.py 92.18% <100.00%> (+0.12%) ⬆️

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 d538c7b...7109ee3. Read the comment docs.

@katiewest820 katiewest820 requested review from MattWhelan and kolanos May 6, 2021 15:38
lk_stack_status = _get_cf_stack_status(input.session, LICENSE_KEY_STACK_NAME)
lk_stack_status = _get_cf_stack_status(
input.session, LICENSE_KEY_STACK_NAME, input.nr_account_id
)
Copy link
Contributor

Choose a reason for hiding this comment

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

A user who has an existing secret with no outputs (because they used an older CLI) will show up as having the stack, and will see the warning, right? We should probably think about how to avoid having them see the warning forever.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not totally following you here. Also, Is that something we want to solve in this ticket?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is an upgrade scenario: user has an existing, working install, and they do an upgrade. They should not get a warning message. So the test case would look like:

  1. Install a region with CLI version 0.5.5.
  2. Run an upgrade with CLI version
  3. Ensure that this happy-path upgrade doesn't generate a warning message.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, speaking of versions, we should change the version in setup.py to 0.6.0 or something :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh yes, I'm tracking now.

@katiewest820 katiewest820 requested a review from MattWhelan May 7, 2021 17:46
@katiewest820 katiewest820 dismissed MattWhelan’s stale review May 10, 2021 16:16

Implemented requested changes

@katiewest820 katiewest820 requested a review from MattWhelan May 10, 2021 20:02
@katiewest820 katiewest820 merged commit 98fa2be into master May 10, 2021
@kolanos kolanos deleted the kwest/lambda-1197 branch November 22, 2021 21:07
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