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

Check if file is in archive before trying to build path #2474

Closed
wants to merge 4 commits into from

Conversation

KennethSamael
Copy link

Addresses #2461

Just a minor reordering that should ensure no new directories are created for files that are already in archive.

@mikf
Copy link
Owner

mikf commented Apr 7, 2022

If only it were that easy ...

This obviously works for this particular problem, but it breaks other things down the line. Each line is somewhat dependent on the others happening first and especially set_filename() has way too many side effects.

There already was a similar issue quite some time ago (#722) and I tried putting the directory creation right before opening a file where it should be, but that didn't work and so I settled for what we have right now.

build_path() has to somehow be decoupled from set_filename(), or at least the call to makedirs():

self.build_path()

os.makedirs(self.realdirectory, exist_ok=True)

@KennethSamael
Copy link
Author

I feared that, but I just couldn't imagine a situation where some code needs to run before being allowed to skip the download.

In that case, my second thought is that setting pathfmt._create_directory to false is equivalent to decoupling. As long as there's no situation where a hooked function requires a directory to exist for a file that won't be downloaded, the solution could still be simple.

mikf added a commit that referenced this pull request May 4, 2022
Only call os.makedirs() when an open() call
fails with a FileNotFoundError

Might cause errors with some post processors etc,
but for now I was unable to find any.
@mikf
Copy link
Owner

mikf commented Jun 7, 2022

This has been implemented since v1.22.0 with commit 99cb287.

It caused some problems when using the part-directory functionality, but those could be resolved (#2576). All in all, this caused a lot less issues than expected.

@mikf mikf closed this Jun 7, 2022
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