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.

Sign up for GitHub

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

Jump to bottom

cdk context --reset <number> does not work #3033

Closed
1 task done
strax opened this issue Jun 24, 2019 · 4 comments · Fixed by #10619 or #10753
Closed
1 task done

cdk context --reset <number> does not work #3033

strax opened this issue Jun 24, 2019 · 4 comments · Fixed by #10619 or #10753
Assignees
@shivlaks
Labels
bug This issue is a bug. effort/medium Medium work item – several days of effort in-progress This issue is being actively worked on. p2 package/tools Related to AWS CDK Tools or CLI
Milestone
CLI

Comments

@strax
Copy link
Contributor

strax commented Jun 24, 2019

  • I'm submitting a ...

    • 🪲 bug report

  • What is the current behavior?
    Running cdk context outputs a table with indices for the current context entries. On the bottom of the output there's a text Run cdk context --reset KEY_OR_NUMBER to remove a context key. It will be refreshed on the next CDK synthesis run.. However, running cdk context --reset 1 will error with:

    No context key with number: 1

  • What is the expected behavior?
    Resetting a context entry works when passed a number.

  • Please tell us about your environment:

    • CDK CLI Version: 0.35.0

  • Other information
    I debugged this a bit and it seems that regardless of the amount of context entries, the index to key mapping will always be [[1, 'bags']]; I'm guessing the context is now wrapped in a distinct Context structure which the reset functionality does not take into account.
@NGL321 NGL321 added bug This issue is a bug. package/tools Related to AWS CDK Tools or CLI needs-triage This issue or PR still needs to be triaged. needs-reproduction This issue needs reproduction. and removed needs-triage This issue or PR still needs to be triaged. labels Jun 25, 2019
@eladb eladb assigned shivlaks and unassigned shivlaks Aug 12, 2019
@shivlaks shivlaks removed the needs-reproduction This issue needs reproduction. label Oct 9, 2019
@shivlaks shivlaks added the p2 label Oct 31, 2019
@cynicaljoy
Copy link

still true as of version CDK CLI Version: 1.44.0

@shivlaks shivlaks added the effort/medium Medium work item – several days of effort label Aug 7, 2020
@rix0rrr rix0rrr added this to the [CLI] Soon milestone Aug 13, 2020
@enobufs
Copy link

enobufs commented Sep 22, 2020

I have just come across this bug using 1.54.0.

@diesal11 diesal11 mentioned this issue Oct 1, 2020
Merged
@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Oct 2, 2020
@mergify mergify bot closed this as completed in #10619 Oct 5, 2020
mergify bot pushed a commit that referenced this issue Oct 5, 2020
@diesal11
fix(cli): cdk context --reset <number> does not work (#10619)
3f62752 
*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Fixes #3033
@github-actions
Copy link

github-actions bot commented Oct 5, 2020

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@NetaNir NetaNir mentioned this issue Oct 7, 2020
Merged
mergify bot pushed a commit that referenced this issue Oct 7, 2020
@NetaNir
chore(cli): "cdk context" is broken (#10751)
4864fa0 
This [commit](#10619), while fixed the problem mentioned, broke the simplest functionally of:
```
cdk context
```
Now throws:

```
Cannot read property 'sort' of undefined
```
This PR reverts that change, and adds a simple unit test that will allow detecting this kind of breakage in unit tests.

We still need to fix the issue in the PR, opening the orginal [issue](#3033). 


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@NetaNir NetaNir reopened this Oct 7, 2020
diesal11 pushed a commit to diesal11/aws-cdk that referenced this issue Oct 7, 2020
@NetaNir @diesal11
chore(cli): "cdk context" is broken (aws#10751)
f0efab4 
This [commit](aws#10619), while fixed the problem mentioned, broke the simplest functionally of:
```
cdk context
```
Now throws:

```
Cannot read property 'sort' of undefined
```
This PR reverts that change, and adds a simple unit test that will allow detecting this kind of breakage in unit tests.

We still need to fix the issue in the PR, opening the orginal [issue](aws#3033).

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@diesal11 diesal11 mentioned this issue Oct 7, 2020
Merged
@mergify mergify bot closed this as completed in #10753 Oct 19, 2020
mergify bot pushed a commit that referenced this issue Oct 19, 2020
@diesal11
fix(cli): cdk context --reset <number> does not work (#10753)
2f3a167 
This PR fixes #3033 while also addressing the issues that arose from #10619

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@shivlaks shivlaks

Labels
bug This issue is a bug. effort/medium Medium work item – several days of effort in-progress This issue is being actively worked on. p2 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Milestone
CLI
8 participants
@cynicaljoy @rix0rrr @strax @enobufs @NGL321 @NetaNir @SomayaB @shivlaks