feat: make cli option to prevent file removal after receiving errors.… #26
Conversation
… integrate option into Config & ClientConfig and populate to Worker applying required behavior
src/client_config.rs
Outdated
| @@ -130,6 +136,7 @@ impl ClientConfig { | |||
| println!(" -u, --upload\t\t\t\tSets the client to upload mode, Ignores all previous download flags"); | |||
| println!(" -d, --download\t\t\tSet the client to download mode, Invalidates all previous upload flags"); | |||
| println!(" -rd, --receive-directory <DIRECTORY>\tSet the directory to receive files when in Download mode (default: current working directory)"); | |||
| println!(" --dont-clean\t\t\t\tWill prevent client from deleting files after receiving errors."); | |||
There was a problem hiding this comment.
Can we make the help text "Prevent client from deleting files after receiving errors" (no ., no Will) in order for it to be more consistent with the remainder?
There was a problem hiding this comment.
Yes of course. I spent 15 minutes trying to came up with description and not making bool named "dont-clean"
There was a problem hiding this comment.
haha, maybe something like --keep-on-error is more descriptive? What do you think?
There was a problem hiding this comment.
--keep-on-error
Done
|
Done |
|
I think the tests still use the old flag, that's why it is failing |
|
Damn I forgot to update tests |
|
Why this is hidden by command line flag? Shouldn't it be default behavior to not delete files when user was downloading, not actually deleting stuff? |
|
This is done when receiving a file fails, so a client sending a file and if there's an error, a cleanup happens, since the file is not complete. This is actually the behaviour of most TFTP servers. |
So, I was tinkering with some embedded device which had only busybox and TFTP client. That time I realized I need minimal but feature-able TFTP server, so I looked up and found your project. Everything was going OK until I stumbled upon troubles with dumping block devices. You see, when program reaches end of available block device, like /dev/mtdblock4, it will throw IO error instead of usual EOF, and when error happens the TFTP daemon deletes file. This is not what I wanted and I realized there is no way to disable file deletion. In the end of the day dump was OK and file deletion is unnecessary in this case.
In this PR I made
Config/ClientConfigparse--dont-cleanoption, which then affects value ofclean_on_errorvariable inConfig/ClientConfigstructs. When instantiatingWorker, there is new param which isclean_on_error. I had to pass it intoWorkerstruct, otherwise I would be stuck with global variable or something.I also changed tests and checked them to work with new parameter.
Best regards,