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

Make rockon-delete compatible with duplicate RockOn model entries #2549 #2552

Conversation

FroggyFlox
Copy link
Member

Fixes #2549
@phillxnet, @Hooverdan96, ready for review.

The rockon-delete currently only supports the "normal" case of a single RockOn model entry. In case one of these entries is duplicated, however, it currently fails.

This Pull Request (PR) thus adapts the current logic to get all entries (0, 1, or more) with a given name.

Demonstration

The example laid out by @Hooverdan96 in #2549 was followed: create a local Rock-On named Scrutiny.local and update the list of available Rock-Ons in the webUI. Then, manually duplicate the RockOn model entry. We end up with the following:

rockdev:/opt/rockstor # psql -U rocky -d storageadmin -c "SELECT id,name,state,status,taskid FROM storageadmin_rockon WHERE name = 'Scrutiny.local';"
Password for user rocky: 
 id |      name      |   state   | status  | taskid 
----+----------------+-----------+---------+--------
 87 | Scrutiny.local | available | stopped | 
 85 | Scrutiny.local | available | stopped | 
(2 rows)

We then run the updated script:

rockdev:/opt/rockstor # poetry run delete-rockon Scrutiny.local
The metadata for the Rock-On named Scrutiny.local (id: 82) has been deleted from the database
The metadata for the Rock-On named Scrutiny.local (id: 84) has been deleted from the database

Verify it is gone:

rockdev:/opt/rockstor # poetry run delete-rockon Scrutiny.local
No Rock-On named Scrutiny.local was found in the database

rockdev:/opt/rockstor # psql -U rocky -d storageadmin -c "SELECT id,name,state,status,taskid FROM storageadmin_rockon WHERE name = 'Scrutiny.local';"
Password for user rocky: 
 id | name | state | status | taskid 
----+------+-------+--------+--------
(0 rows)

The script still works for a Rock-On with a single entry (normal case):

rockdev:/opt/rockstor # poetry run delete-rockon Jellyfin
The metadata for the Rock-On named Jellyfin (id: 83) has been deleted from the database

Note that we also have verification by the two forum users who reported this issue that the updated script resolved their duplicate RockOn entries:

Testing

rockdev:/opt/rockstor # cd src/rockstor/ && poetry run django-admin test
Creating test database for alias 'default'...
Creating test database for alias 'smart_manager'...
System check identified no issues (0 silenced).
.............................................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 253 tests in 12.550s

OK

…kstor#2549

The rockon-delete currently only supports the "normal" case of a single
RockOn model entry. In case one of these entries is duplicated, however,
it currently fails.

This commit thus adapt the current logic to get all entries (0, 1, or more)
with a given name.

Further detail script's usage (@Hooverdan96's idea).

Black formatting.
@Hooverdan96
Copy link
Member

Most excellent!

Copy link
Member

@phillxnet phillxnet left a comment

Choose a reason for hiding this comment

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

@FroggyFlox Thanks for seeing to this task and raising our game re rock-ons delete.
Confirmed at least the happy paths re:

rleap15-4:/opt/rockstor # poetry run delete-rockon Scrutiny.local
No Rock-On named Scrutiny.local was found in the database
rleap15-4:/opt/rockstor # poetry run delete-rockon "Netdata (official)"
The metadata for the Rock-On named Netdata (official) (id: 71) has been deleted from the database

@phillxnet
Copy link
Member

@FroggyFlox My apologies for taking so long to see to this pull request. Merging directly.

@phillxnet phillxnet merged commit a5b2da5 into rockstor:testing May 23, 2023
@FroggyFlox FroggyFlox deleted the 2549_Custom_Rockon_management_deletion_script_enhancement branch May 27, 2023 15:40
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.

None yet

3 participants