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

feat(toolkit): add 'cdk context' command #1169

Merged
merged 5 commits into from
Nov 15, 2018
Merged

Conversation

rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented Nov 14, 2018

Add a command to view and manage cached context values.

Fixes #311.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

Add a command to view and manage cached context values.

Fixes #311.
Viewing and managing context
###########################

Context is used to retrieve things like Availability Zones available to you, or
Copy link
Contributor

Choose a reason for hiding this comment

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

"Availability zone available" sounds weird

│ │ 64-gp2:region=us-east-1 │ │
└───┴────────────────────────────────────────────────────┴────────────────────────────────────────────────────┘

Run cdk context --invalidate KEY_OR_NUMBER to invalidate a context key. It will be refreshed on the next CDK synthesis run.
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. I wonder if --clear or --reset are more intuitive in this context
  2. Can you invalidate everything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--clear actually clears everything. cdk context --help will show it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will call it reset.

return 0;
}

function listContext(context: any) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a --json flag that will output the context as JSON for machines?

}

// Unset!
if (!(key in context)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can probably be a no-op (think scripts that want to ensure that a context is invalidated)

return key;
}
}
throw new Error(`No context key with number: ${n}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add --force and this will be no-op (similar to rm -f)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is for a semi-interactive usage, where copying the key is a bother and you simply supply the number. Blindly deleting by key # is dangerous, given that the numbering is not unique.

But yeah, just deleting a nonexistant key will be a no-op.

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right of course!

Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

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

One last question... can you write a small integration test for this and make sure you execute all previous tests as well?

@rix0rrr
Copy link
Contributor Author

rix0rrr commented Nov 15, 2018

Yas

@rix0rrr rix0rrr merged commit 2db536e into master Nov 15, 2018
@rix0rrr rix0rrr deleted the huijbers/context-ui branch November 15, 2018 16:23
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Toolkit: support refreshing environmental context
3 participants