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: allow deletion of multiple resources at once #719

Merged
merged 3 commits into from
Apr 3, 2024
Merged

Conversation

phm07
Copy link
Contributor

@phm07 phm07 commented Mar 20, 2024

Currently, the CLI only allows you to delete one resource at a time. This can be time consuming and tedious if you want to delete multiple resources, because you have to wait for each resource to be deleted before you can delete the next one.
This PR adds the ability to delete multiple resources at once by passing them to the delete subcommand as multiple positional arguments, e.g. hcloud server delete server1 server2 .... It also adds tests to verify that this works as intended.

@phm07 phm07 added the feature label Mar 20, 2024
@phm07 phm07 self-assigned this Mar 20, 2024
@phm07 phm07 requested a review from a team as a code owner March 20, 2024 12:18
Copy link

codecov bot commented Mar 20, 2024

Codecov Report

Attention: Patch coverage is 46.66667% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 58.43%. Comparing base (6cea4cd) to head (8327b12).

Files Patch % Lines
internal/cmd/base/delete.go 46.66% 5 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #719      +/-   ##
==========================================
- Coverage   58.44%   58.43%   -0.02%     
==========================================
  Files         180      180              
  Lines        6507     6510       +3     
==========================================
+ Hits         3803     3804       +1     
- Misses       2094     2096       +2     
  Partials      610      610              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

if err != nil {
return err
}
for _, idOrName := range args {
Copy link
Member

Choose a reason for hiding this comment

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

The current code deletes the resources in series. Doing it in parallel would be faster and we could wait for all actions at once.

I am fine with merging it as it is and leaving this for the future to optimize.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doing it this way we can guarantee the order of operations (which is also important for the tests for example). I agree that we can leave it like this for now.

@phm07 phm07 merged commit 3b896fe into main Apr 3, 2024
4 checks passed
@phm07 phm07 deleted the delete-multiple branch April 3, 2024 11:48
phm07 pushed a commit that referenced this pull request Apr 3, 2024
🤖 I have created a release *beep* *boop*
---


##
[1.43.0](v1.42.0...v1.43.0)
(2024-04-03)


### Features

* allow deletion of multiple resources at once
([#719](#719))
([3b896fe](3b896fe))
* improve missing argument error messages
([#711](#711))
([e7f9e74](e7f9e74))
* **server:** allow JSON & YAML output in reset-password
([#716](#716))
([373287b](373287b)),
closes [#715](#715)


### Bug Fixes

* removing last rule from firewall fails with invalid_input error
([#696](#696))
([acab17c](acab17c))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
phm07 added a commit that referenced this pull request May 23, 2024
This PR builds onto #719 by making the deletion of multiple resources
parallel using the new action waiters introduced in #749.
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 this pull request may close these issues.

2 participants