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

Add scan option to force gallery zip rescan #4113

Conversation

cj12312021
Copy link
Collaborator

@cj12312021 cj12312021 commented Sep 12, 2023

This pull request adds a new scan option to force rescan gallery zips. Currently, the only way to force a zip rescan is to touch the file via the command line, move the file, or rename the file, which aren't great options for most users.

There are a few reasons why a user may want to force a rescan. One of the reasons I've encountered is that when using a script to update the contents of a zip, the script, depending on the libraries used, doesn't actually update the file's modified date, causing Stash to overlook the changes made. Another reason is when a known bug in Stash updates the path of a moved gallery zip but does not update the path of the images contained within the zip.

Here is an image showing this new scan option:
Screenshot 2023-09-11 213619

@cj12312021 cj12312021 added the improvement Something needed tweaking. label Sep 12, 2023
@DingDongSoLong4
Copy link
Collaborator

Simple enough change, just two comments:

  • We don't want users to force rescan on every scan, it should only be used when necessary. So I think the state of the "Force rescan zips" option should not be remembered between scans - it should always be off unless you specifically enable it.
  • For similar reasons, I think the "Force rescan zips" toggle should be at the bottom of the toggle list - it's more like "Overwrite existing files" than "Generate scene covers".

@WithoutPants
Copy link
Collaborator

We can rescan individual galleries currently, but there's no such option to select and scan multiple galleries. Would the latter be a better solution to the underlying issue? I would imagine one would use this to rescan specific galleries, and having it as a scan task option is a bit of a sledgehammer approach.

@cj12312021
Copy link
Collaborator Author

Simple enough change, just two comments:

  • We don't want users to force rescan on every scan, it should only be used when necessary. So I think the state of the "Force rescan zips" option should not be remembered between scans - it should always be off unless you specifically enable it.
  • For similar reasons, I think the "Force rescan zips" toggle should be at the bottom of the toggle list - it's more like "Overwrite existing files" than "Generate scene covers".

I completely agree with both suggestions. The possibility of having the state of "Force rescan zips" reset itself to false each time didn't cross my mind until you mentioned it. I also only had "Force rescan zips" at the top because, at first glance, it thought things were placed alphabetically.

@cj12312021
Copy link
Collaborator Author

We can rescan individual galleries currently, but there's no such option to select and scan multiple galleries. Would the latter be a better solution to the underlying issue? I would imagine one would use this to rescan specific galleries, and having it as a scan task option is a bit of a sledgehammer approach.

There is likely a longstanding bug with the existing option to rescan individual galleries, as it has never worked for me with my zips. If the option worked, ideally, it would also offer a way to scan everything in a directory or even everything in Stash if a user isn't sure which files may have been affected by the existing bug or any others in the future. Users affected by the bug I mentioned earlier most likely moved a directory of files rather than a select set of files. In my case, one of the moved directories contains 28,013 galleries, so the 1,000 file selection limitation would be inconvenient when addressing this. Having this as a scan option seemed like the ideal location. Concerns around mistakenly using this option should be addressed with DDSL's suggestions.

@DingDongSoLong4
Copy link
Collaborator

DingDongSoLong4 commented Sep 12, 2023

No idea how difficult it would be, but perhaps this option should be made into a more generic "Force rescan" option, that works for all files, not just zips. In other words, have all existing files be rescanned, recalculating their fingerprints, even if their modtimes haven't changed.

The existing Rescan option would then have "Force rescan" enabled, to ensure that the file is rescanned from scratch.

A bulk rescan option is a good idea, and it should of course work for galleries, images and scenes. I think having the force rescan option on the scan task is still useful though - maybe just to be sure we can display a warning if you do a non-selective scan with the Force rescan option enabled. I imagine if you're using force rescan you'd want to only run it for a specific path and not for the entire library.

Regarding the bug, I imagine the reason is because the rescan will just do a selective scan for that file only - which means that Stash will just see that the modtime hasn't changed and thus won't bother traversing the zip contents at all. This is intentional, as otherwise stash needs to traverse all zip files for every scan - but it does cause this issue. A force rescan option should fix this.

@cj12312021
Copy link
Collaborator Author

Regarding the bug, I imagine the reason is because the rescan will just do a selective scan for that file only - which means that Stash will just see that the modtime hasn't changed and thus won't bother traversing the zip contents at all.

I had a feeling that behavior was related to the mod time not being updated.

No idea how difficult it would be, but perhaps this option should be made into a more generic "Force rescan" option, that works for all files, not just zips. In other words, have all existing files be rescanned, recalculating their fingerprints, even if their modtimes haven't changed.

So currently, this option really only triggers a rescan within gallery zips. It doesn't regenerate thumbnails or recalculate any hashes for existing images it finds in the zip, only new ones. Since zips are unique in the sense that associated files aren't directly accessible, it made sense to have an option specifically for them. Also, I really wouldn't want to recalculate or regenerate anything for existing files with this option. That would add significantly more overhead. Perhaps that capability could be available in a separate option.

@DingDongSoLong4
Copy link
Collaborator

Alright you've convinced me that both options are useful.

We should avoid adding too many options to the scan page though - there are enough there already. Maybe one (or both) of these options should be API-only (ie not in the UI), at least for now.

And just to clarify, by "all existing files be rescanned" I mean that only the oshash/checksum would be recalculated - thumbnails/phashes/previews/etc wouldn't be affected. Essentially it's just as if you touched all the files in the folder.

@cj12312021
Copy link
Collaborator Author

cj12312021 commented Sep 12, 2023

The clarification was helpful since I did misunderstand. It shouldn't add as much overhead as I initially thought. I'm fine with consolidating the option with that info in mind. I do think the consolidated option should remain visible for less technical users affected by the bug I mentioned in the original comment or others.

@cj12312021 cj12312021 added feature Pull requests that add a new feature and removed improvement Something needed tweaking. labels Sep 13, 2023
@cj12312021
Copy link
Collaborator Author

My latest push includes documentation and a tooltip for this option to make its use case clear. My push from yesterday evening made the option more generic, so it applies to all files instead of just galleries. Yesterday's push also resets this option to false each time a user visits the page and moves the option to the very bottom of the list.

@Ksrx01
Copy link
Contributor

Ksrx01 commented Oct 21, 2023

The initial description of this feature would be very useful to me right now.
I scanned several hundred zip-galleries with the "generate thumbnails" option set to off,
and would need to touch each single gallery manually to generate thumbs now.

It doesn't regenerate thumbnails or recalculate any hashes for existing images it finds in the zip

Unfortunately, if I understand this correctly, the scope of the feature changed to a point where the above use-case wouldn't apply anymore. Am I correct?

@WithoutPants
Copy link
Collaborator

Yes, I think your use case might be better suited for a change to the generate task.

@DingDongSoLong4
Copy link
Collaborator

@Ksrx01 What stash normally does is if a file's modtime has not changed, it doesn't bother checking whether the actual file contents (hash) has changed. Both versions here are just forcing stash to always check whether the hash has changed, bypassing the modtime check - the only difference is that the original version only bypasses that check for zip files whereas the newer version bypasses the check for all files.

They are effectively an alternative to manually touching every file to change its modtime. If touching every zip file solves your problem, then either version of this PR should as well. However, as @WithoutPants mentions, a much better fix for your issue would be an additional option in the Generate task (#4189).

@Ksrx01
Copy link
Contributor

Ksrx01 commented Oct 22, 2023

Thank you for the heads up and info. Going to wait for #4189.

@WithoutPants
Copy link
Collaborator

I think the underlying use cases were probably addressed, eliminating the need for this feature in most cases. I can see the utility of this, but I'm not sure if it necessarily needs to be given a place in the UI. I'd expect this to be used pretty infrequently and by advanced users, and triggering such a scan could probably be done using the playground instead.

@cj12312021
Copy link
Collaborator Author

I'll admit I haven't had a need for it since the issue that prompted the change, but this is partly because I've been more wary of how I use my galleries in Stash, knowing that the feature isn't perfect. I'm fine with tucking this toggle away via the playground.

@WithoutPants WithoutPants added this to the Version 0.26.0 milestone May 20, 2024
@WithoutPants WithoutPants merged commit 0fa71be into stashapp:develop May 20, 2024
2 checks passed
@cj12312021
Copy link
Collaborator Author

I completely forgot to return to this PR. Thanks for updating it to remove the setting from the UI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Pull requests that add a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants