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

[FR]: config creates full path if it doesn't exist #271

Closed
2 tasks
jonoff opened this issue Apr 22, 2023 · 13 comments · Fixed by #273
Closed
2 tasks

[FR]: config creates full path if it doesn't exist #271

jonoff opened this issue Apr 22, 2023 · 13 comments · Fixed by #273
Assignees
Labels
feature request New feature or request status:added-to-develop Feature Request or Bug Fix is in Develop

Comments

@jonoff
Copy link

jonoff commented Apr 22, 2023

Is your feature request related to a problem? Please elaborate.

The recycle bin config variable is always treated as a subfolder of remote_dir, and when specifying a full path it is stripped to it's basename, which becomes the last folder of a full path:

self.remote_dir, os.path.basename(self.data["directory"]["recycle_bin"].rstrip(os.sep))

Describe the solution you'd like

The example config and comment indiciate it's used an the full path
# <OPTIONAL> recycle_bin var: </your/path/here/> # Path of the RecycleBin folder. Default location is set to remote_dir/.RecycleBin

Instead of joining paths, just treat it as a full path when specified.

Does your solution involve any of the following?

  • New config option
  • New command option

Describe alternatives you've considered

The default may be a subfolder, but why limit it in this way?

Who will this benefit?

Any custom configurations

Additional Information

No response

@jonoff jonoff added the feature request New feature or request label Apr 22, 2023
@bobokun
Copy link
Collaborator

bobokun commented Apr 22, 2023

It's not limited. If you specify full path it should work. Did you test and verify it's not working with the full path?

@jonoff
Copy link
Author

jonoff commented Apr 22, 2023

Yes, checking a recent log, it matches the code as it's only using the "basename" instead of /full/path/to/basename_bin
I see

Moved 1 files to [remote_dir]/[basename_bin]

@jonoff
Copy link
Author

jonoff commented Apr 22, 2023

Looks like orphaned directory also is limited to be
a sub-directory, but I did not test this one:

if self.commands["rem_orphaned"]:
if "orphaned_dir" in self.data["directory"] and self.data["directory"]["orphaned_dir"] is not None:
default_orphaned = os.path.join(
self.remote_dir, os.path.basename(self.data["directory"]["orphaned_dir"].rstrip(os.sep))

@bobokun
Copy link
Collaborator

bobokun commented Apr 22, 2023

The code that you shared is only specifying the default recycle path and default orphaned path. Those are being used as default when the user does not specify. I just tested this and it's working correctly with specifying the exact path. It will use the default if the folder you specified does not exist or is not accessible. That's probably why you are seeing it use the default recycle directory.

@jonoff
Copy link
Author

jonoff commented Apr 22, 2023

Interesting, most likely permission issue on my setup I'll check into.

Instead of the original default being used, since it overrides the default to include the basename of what I specified, this wasn't what I expected when the full path is inaccessible and I didn't see any related error.

@bobokun
Copy link
Collaborator

bobokun commented Apr 22, 2023

yeah I can add additional logs. In case you're wondering, the code that does the check to see if the folder exists is found here:

qbit_manage/modules/util.py

Lines 181 to 183 in 9e621e3

elif var_type == "path":
if os.path.exists(os.path.abspath(data[attribute])):
return os.path.join(data[attribute], "")

@jonoff
Copy link
Author

jonoff commented Apr 22, 2023

I'm also wondering what the reasoning is to override the default ".RecycleBin" to instead point to the basename of a specified path, if the full path is not accessible.

Permissions look correct but looking deeper now.

bobokun added a commit that referenced this issue Apr 22, 2023
@bobokun
Copy link
Collaborator

bobokun commented Apr 22, 2023

I'm also wondering what the reasoning is to override the default ".RecycleBin" to instead point to the basename of a specified path, if the full path is not accessible.

It's because of #77, Not everyone might want the name .RecycleBinas the folder name. Taking the basename will keep the folder name of your preferred RecycleBin that you define in your configuration and put it in your root directory if the location you specified cannot be found.

What is currently set in your config.yml under directory?

@jonoff
Copy link
Author

jonoff commented Apr 23, 2023

I was trying to reuse my NAS' recycle bin path. Dug a little deeper and looks like my issue resulted from the difference of how the full path is treated vs. the default: For the full path check, it's never created and only checked for existence, but the basename path is created when it doesn't exist already. I was assuming the full path would be created as well. the basename path being created silently led to my confusion.

qbit_manage/modules/util.py

Lines 181 to 185 in 9e621e3

elif var_type == "path":
if os.path.exists(os.path.abspath(data[attribute])):
return os.path.join(data[attribute], "")
else:
message = f"Path {os.path.abspath(data[attribute])} does not exist"

This warning message used to be silently dropped before your new commit, since one of the branches returned before printing any message.

I'd consider this issue resolved with 2 more minor improvements:

  1. Update the documentation to include an example of using the basename vs full path
  2. run makedirs on full paths and surface any errors during creation, before resorting to using/creating default (basename) paths if the full ones don't exist, treating both similar to:

706821f#diff-c39b942d8f8620d46d314db8301189b8d6195fc97aedbeb124a33694b738d69cR202-R203

bobokun added a commit that referenced this issue Apr 23, 2023
@jonoff jonoff changed the title [FR]: Allow Recycle bin to specify the full path [FR]: config creates full path if it doesn't exist Apr 23, 2023
@bobokun
Copy link
Collaborator

bobokun commented Apr 23, 2023

Can you test the new develop branch? It should create the Recycle Bin folder now for the path you specified.

@bobokun bobokun added the status:added-to-develop Feature Request or Bug Fix is in Develop label Apr 23, 2023
@jonoff
Copy link
Author

jonoff commented Apr 24, 2023

Can you test the new develop branch? It should create the Recycle Bin folder now for the path you specified.

I'm running hotio's container image, I can try the nightly tag but doesn't seem to have a develop tag.

@bobokun
Copy link
Collaborator

bobokun commented Apr 24, 2023

Yes, hotios nightly tag points to develop branch

@jonoff
Copy link
Author

jonoff commented Apr 24, 2023

Yep, just verified on :nightly, it created Recycle Bin at the full path specified.

@bobokun bobokun mentioned this issue Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request status:added-to-develop Feature Request or Bug Fix is in Develop
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants