lbryio / lbry-sdk

The LBRY SDK for building decentralized, censorship resistant, monetized, digital content apps.
https://lbry.com
MIT License
7.19k stars 482 forks source link

Daemon.jsonrpc_file_delete: new `--what` parameter to control what to delete #3430

Closed belikor closed 3 years ago

belikor commented 3 years ago

The --what parameter can be used to specify what to delete: the media file only, the blobs only, or both, deleting completely the downloaded stream.

lbrynet file delete --claim_name=my-claim --what=file
lbrynet file delete --claim_name=my-claim --what=blobs
lbrynet file delete --claim_name=my-claim --what=both

If no argument is used, this is equivalent to --what=blobs, which is the original behavior.

lbrynet file delete --claim_name=my-claim

The old argument, --delete_from_download_dir, works as --what=both.

lbrynet file delete --claim_name=my-claim --delete_from_download_dir
lbrynet file delete --claim_name=my-claim --what=both

Personally, I think we could deprecate completely this --delete_from_download_dir parameter.


In general, we extend the delete method of FileManager with the parameter delete_blobs to make the deletion of the blobs optional. This requires adding the argument delete_source to StreamManager.delete.

However, the pylint configuration complains and tells us that there is a mismatch in the arguments of the parent and children classes, so we also need to add the corresponding delete_source argument to SourceManager.delete and TorrentManager.delete.

coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 68.034% when pulling f665edb82a2a5a26e4fb9ba12de04fa16aa1a0ed on belikor:file-delete-what into 2d9e3e18478025ef865ee6f235045fed24a9f8a0 on lbryio:master.

eukreign commented 3 years ago

@lyoshenka this PR involves API changes, please review

lyoshenka commented 3 years ago

does this add anything we don't have today, or is it just a rename of the flags that already exist?

if the latter, what's the upside of this?

belikor commented 3 years ago

does this add anything we don't have today, or is it just a rename of the flags that already exist?

With this new option, we make deleting blobs optional (True by default).

Currently file_delete always deletes the blobs. There are instances where the user would like to delete the media files (mp4, mkv, mp3, etc.) to free space on disk but keep the blobs to continue seeding the content. This is desired for a seedbox, for example.

# Current and new; delete only blobs
lbrynet file delete --claim_name=<name>
lbrynet file delete --claim_name=<name> --what=blobs  # Also without this option

# Current and new; delete blobs and file
lbrynet file delete --claim_name=<name> --delete_from_download_dir
lbrynet file delete --claim_name=<name> --what=both

# New; delete only the file
lbrynet file delete --claim_name=<name> --what=file

So, it is not a rename, it's about giving the user more flexibility. The old option --delete_from_download_dir is kept, and it's just made equal to --what=both.

lyoshenka commented 3 years ago

The SDK should not be responsible for deleting files that users download. They can be deleted directly using normal commands such as rm.

I'm going to close this. We can continue to discuss if you think I missed something.