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

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

Closed
wants to merge 1 commit into from

Conversation

belikor
Copy link
Contributor

@belikor belikor commented Sep 17, 2021

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.

@belikor belikor force-pushed the file-delete-what branch 2 times, most recently from 32ad51b to f665edb Compare September 17, 2021 21:41
@coveralls
Copy link

Coverage Status

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

@eukreign eukreign assigned lyoshenka and unassigned eukreign Sep 18, 2021
… delete

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`.
```
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
```
@eukreign
Copy link
Member

@lyoshenka this PR involves API changes, please review

@lyoshenka
Copy link
Member

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?

@lyoshenka lyoshenka assigned belikor and unassigned lyoshenka Sep 21, 2021
@belikor
Copy link
Contributor Author

belikor commented Sep 22, 2021

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
Copy link
Member

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.

@lyoshenka lyoshenka closed this Sep 23, 2021
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

4 participants