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

Delete error solved #180

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Delete error solved #180

wants to merge 6 commits into from

Conversation

bornagain1981
Copy link

@bornagain1981 bornagain1981 commented Sep 8, 2023

On server.py the delete option accepts only boolean False, without quotes. An error occurred when you're working for the decompression of files on the fly.

@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Merging #180 (c7d630c) into main (c9f33e6) will decrease coverage by 0.02%.
Report is 2 commits behind head on main.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #180      +/-   ##
==========================================
- Coverage   90.57%   90.55%   -0.02%     
==========================================
  Files          24       24              
  Lines        5239     5240       +1     
==========================================
  Hits         4745     4745              
- Misses        494      495       +1     
Flag Coverage Δ
unittests 90.55% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
trollmoves/server.py 67.85% <0.00%> (-0.11%) ⬇️

Copy link
Member

@pnuu pnuu left a comment

Choose a reason for hiding this comment

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

This will need a unit test that checks the three options work:

  • user doesn't define delete in the config -> the default is used
  • user defines delete = True and the file is deleted
  • user defines delete = False and the file is not deleted

The current implementation will fail on the True case.

@@ -945,7 +945,7 @@ def unpack(pathname,
except Exception:
LOGGER.exception("Could not decompress %s", pathname)
else:
if delete.lower() in ["1", "yes", "true", "on"]:
if delete in ["1", "yes", "true", "on"]:
Copy link
Member

@pnuu pnuu Sep 8, 2023

Choose a reason for hiding this comment

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

This won't work if delete is set to any value that evaluates as boolean True.

@bornagain1981
Copy link
Author

@pnuu I traced all the options (1,yes..) back to True. If I use test_server.py on test folder I have no error message

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.

2 participants