search

algoo / preview-generator

generates previews of files with cache management
https://pypi.org/project/preview-generator/
MIT License
228 stars 51 forks source link

Quiet start #109

Closed JulienPalard closed 2 years ago

JulienPalard commented 4 years ago

I moved the builder.check_dependencies() from "start time" (creation of the manager) to runtime (actual use of the builder) to close https://github.com/algoo/preview-generator/issues/88.

But as multiple builders can share some mimetypes, I had to still quietly check some (not all) dependencies at start time, to choose the "best" builder. Anyway this probably means faster load time (not measured).

By "best" builder I mean: If we have 3 builders for jpg, and two of them are missing dependencies, choose the other. If non of them can, still pick one.

Why still picking one? Just so this one can explain at runtime why the build can't happen. This is not optimal because at runtime we're only saying "can't build that because this is missing" but in reality there may exist other alternatives to build the thing thrue other builders. It would be nice to give "Cannot build that, please install this or this or this to fix it".

Just to help me in comparing develop to my branch I also added -v, -vv, and -vvv to the preview command line script.

⚠ this patch may subtly change the behavior of preview-generator

This is because the algorithm to choose a builder for a mimetype changed. Typically, before the patch, to build a jpg preview we used ImagePreviewBuilderPillow, we're now using ImagePreviewBuilderWand.

This is because in the previous version if there were two builders providing the same mimetype, the last one would win. In the current version, the first one wins and only be overwritten by another one if the new one is better (meaning: the old one is missing a dependency while the new one is not).

JulienPalard commented 4 years ago

Some tests were passing before and are no longer passing after my patch I still have to investigate. Do not merge.

I'm currently having an issue testing with libreoffice: https://bugs.documentfoundation.org/show_bug.cgi?id=128220 :(

JulienPalard commented 4 years ago

Found a lot of regressions, most due to the re-ordering of builders.

Wand gets a lot of work now, but it was unable to do most of it.

Also found strange things in the tests, like svg in portrait mode being asserted to be reduced in landscape mode.

We either have to fix all regressions by individually fixing builders. Or I should just give up and try just applying reverse on get_subclasses_recursively to try to mimick the old behavior. Will test this later.

JulienPalard commented 4 years ago

[DO NOT MERGE] this PR currently introduces a lot of regressions that should be fixed first, and I think in separate PRs.

I'll rebase this one on develop once all problems resolved (if possible).

JulienPalard commented 4 years ago

Blocked by #113 #114 #115.

inkhey commented 4 years ago

@JulienPalard I'm working on this PR: https://github.com/algoo/preview-generator/pull/148. I'm not sure how this code should work with your quiet start proposal, This PR currently add a "builder.update_mimetypes_mapping()" in "register_builder" method.