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

verdi code delete falsely claims it actually deleted a node #5842

Closed
sphuber opened this issue Dec 16, 2022 · 3 comments
Closed

verdi code delete falsely claims it actually deleted a node #5842

sphuber opened this issue Dec 16, 2022 · 3 comments
Assignees
Labels
Milestone

Comments

@sphuber
Copy link
Contributor

sphuber commented Dec 16, 2022

It claims that the code is deleted, but it is not actually the case. Seems that the problem is with the PsqlDosBackend.delete_nodes_and_connections that does perform the deletion but changes are not persisted. This is most likely related to recent refactorings of the PsqlDosBackend.transaction method.

@sphuber sphuber added type/bug priority/critical-blocking must be resolved before next release topic/storage labels Dec 16, 2022
@sphuber sphuber added this to the v2.3.0 milestone Dec 16, 2022
@sphuber sphuber self-assigned this Dec 16, 2022
@sphuber
Copy link
Contributor Author

sphuber commented Dec 16, 2022

There is actually a test for this and it passes. I suspect the reason is that the test doesn't actually go exactly through the same route as when calling verdi code delete but it uses the click.TestRunner. This has bitten us in the ass before as these two pathways are not exactly identical. We should really have an alternative for run_cli_command to run through subprocess to simulate the actual pathway that a user would experience.

@sphuber
Copy link
Contributor Author

sphuber commented Dec 21, 2022

The scope of this problem is way bigger than thought. The problem is that changes to the session in the PsqlDosBackend are no longer committed correctly. So essentially most (if not all) verdi commands that mutate state (create groups, delete nodes, etc.) do not persist the changes. The same goes for an interactive shell. If you run Group(label='test').store() in a verdi shell and then exit, the group won't exist.

@sphuber sphuber modified the milestones: v2.3.0, v2.2.1 Dec 22, 2022
@sphuber
Copy link
Contributor Author

sphuber commented Dec 22, 2022

This is fixed in #5851 by reverting the offending commits and will be released with v2.2.1

@sphuber sphuber closed this as completed Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant