Import CDBOOP instructions unclear, resulting in failed migration

[Ainaemaet]

Hello. Thank you for the updates.

The issue I'm having is the instructions for CDBoop data migration say only "Support migration from cdb-boop/ComfyUI-Model-Manager/main", and the settings menu only "Migrate information from cdb-boop/main".

When we install @cdb-boop's fork, it installs to 'ComfyUI-Model-Manager' just as this one does, and generally will just put '.disabled' at the end of the folder name when swapping to this one.

I've tried both leaving the folder name to his cloned repo as "/custom_nodes/ComfyUI-Model-Manager.disabled" as well as changing the directory structure of his fork to "ComfyUI/custom_nodes/cdb-boop/ComfyUI-Model-Manager/main" (files in 'main') - but in both configurations the migration fails.

Is there a specific place these files should be other than this? And if that is the case, could you please consider updating the instructions to clearly reflect it?

Thank-you!

[hayden-fr]

I don't know what data you are missing, or you can give me more detailed information about the migration failure.

In fact, switching from cdb-boop/ComfyUI-Model-Manager to hayden-fr/ComfyUI-Model-Manager does not require extra work. The new UI itself is compatible with the old format, but it lacks some meta information.

The old format of information looks like this:

a_sd_xl_base_1.0.jpg
a_sd_xl_base_1.0.json
a_sd_xl_base_1.0.safetensors
a_sd_xl_base_1.0.txt
a_sd_xl_base_1.0.url

Among them, a_sd_xl_base_1.0.json and a_sd_xl_base_1.0.url are file meta information unique to cdb-boop.

The new format of information looks like this:

a_sd_xl_base_1.0.md
a_sd_xl_base_1.0.safetensors
a_sd_xl_base_1.0.webp

The new format merges a_sd_xl_base_1.0.json a_sd_xl_base_1.0.url into a_sd_xl_base_1.0.txt and rename a_sd_xl_base_1.0.txt to a_sd_xl_base_1.0.md. This is also the implementation logic of the migration function.

So, even if you do not migrate the data, you can still display a_sd_xl_base_1.0.jpg and a_sd_xl_base_1.0.txt, but the meta information will be missing. Because those data is stored in the model folder and not the custom_nodes folder.

Of course, there is another piece of data that has not been migrated, which is APIKey. I think this can be migrated manually. If program migration is required, I can also add it.

[hayden-fr]

If the problem of migrating data cannot be solved, you can choose to Setting -> ModelManager -> Scan Files -> Full Scan.

[Ainaemaet]

Ok, I had assumed the model info/previews must be stored somewhere in the ModelManager folder and I just wasn't seeing them; given that I definitely have a file structure that should work (I have the files alongside my models).

I don't mind updating the file info myself as-necessary when I use them - it's mostly the previews I want there, so I can have a visual of what I'm looking for.

I've tried pouring over the code of both repos, but can't find where things are going wrong...

Most of my previews are modelname.preview.png (downloaded through a powerful Auto1111 extension I became accustomed to a couple years ago called 'CivitaiBrowser+'), which cdboop seems to pick up without issue; could that be the problem?

And if so, can you help direct me to where and what I would need to change to have it pick up .pngs for migration? I've tried finding it, but the only places I see 'migrate' in the code have me absolutely baffled trying to figure out.

Thank you.

[Ainaemaet]

Looking in 'services.py' it appears .png shouldn't be an issue - really not sure what the problem is.

If I can make a suggestion though, it looks like the code is designed to 'clean up' by removing the old previews after changing them to .webp; could you consider changing this to make it optional (a checkbox in the menu), and/or at the very least give a warning in the instructions and menu?

I'm actually feeling pretty lucky it didn't work for me yet, because had I not checked the code I wouldn't have known and would have lost all of my .preview.png's, which I need for other purposes.

[hayden-fr]

Most of my previews are modelname.preview.png (downloaded through a powerful Auto1111 extension I became accustomed to a couple years ago called 'CivitaiBrowser+'), which cdboop seems to pick up without issue; could that be the problem?

Yes, the problem should be here. I have made compatibility, but I have not tested it. It seems that pictures like .preview.png cannot be recognized.

Please wait, I think I can fix it right now.

[hayden-fr]

It's my mistake. I just discovered when checking the code that the compatibility with .preview.png image files was removed in a previous modification. I will fix it immediately.

[cdb-boop]

if os.path.isfile(preview_path) and preview_path != new_preview_path:
    utils.print_info(f"Migrate preview image from {fullname}")
    with Image.open(preview_path) as image:
        image.save(new_preview_path, format="WEBP")
    os.remove(preview_path)

To follow up with what @Ainaemaet said, am I missing some context? Is this actually deleting existing preview files and replacing them with WEBP? What if I have important data embedded inside a PNG or have a PNG file that is unreproducible? Not to mention, PNG is lossless, while WEBP is lossy.

[hayden-fr]

I admit that my consideration wasn't thorough enough for the image with embedded data. The reason for choosing WEBP is precisely because it is a compressed format. PNG, being a lossless format, can result in larger file sizes, which may impose a certain burden on network requests. For preview images, I believe using WEBP is sufficient.

[cdb-boop]

Yes, when I made "version 1" I was aware of that. Obviously webp is more efficient for internet file transfer over png. However, you are not addressing my point. Maybe I'm missing some context, but is the code destroying user data irreversibly?

[Ainaemaet]

@hayden-fr Glad it is nothing too crazy, I spent way too much time looking at code today 😆.

Looking forwards to trying the fix (thank you)! 😊

About the 'cleanup' code that deletes the previous images... It's easy enough to deal with on my end, I can just temporarily back-up all my preview images, or try to comment out the bits that remove them - but I could see it potentially being a serious issue for people who might not know what they're doing.

I appreciate all the hard work you all have put in to this project - it really makes ComfyUI live up to its name a bit more lol.

[hayden-fr]

Yes, when I made "version 1" I was aware of that. Obviously webp is more efficient for internet file transfer over png. However, you are not addressing my point. Maybe I'm missing some context, but is the code destroying user data irreversibly?

You're right, deleting old files does cause irreversible damage.

Initially, the purpose of deleting old files was to make it easier for me to find updated files, especially for preview images.

eg. the original preview image of a_sd_xl_base_1.0.safetensors is a_sd_xl_base_1.0.png, and then I updated it to a_sd_xl_base_1.0.jpg, without deleting the old image, there will be two images in the folder. It is impossible for the program to accurately obtain which is the correct preview image. If the creation time of the two files is compared, It may increase the time it takes to scan files, and this is uncontrollable.

There is a new proposal:

Do not delete any files. When scanning, choose .webp as its default preview. If not, select the first one from the remaining optional pictures.

[cdb-boop]

Do not delete any files. When scanning, choose .webp as its default preview. If not, select the first one from the remaining optional pictures.

This sounds more reasonable.

And if you decide you really don't want to recompute previews on demand, create a cache folder or something you check first, in which case, just make sure the cached previews are actually up-to-date.

[hayden-fr]

I've considered caching folders before, but I didn't feel like it was a good idea. If the user manually organizes the contents of the model folder, the cached information will be invalid, thus causing some unknown errors.

Adding a clear cache button in settings to resolve it? ?

[cdb-boop]

Yeah, I don't like caching either. That's why I didn't do it.

I think can be done, but it would require a lot of careful thought. Maybe simply checking the filesystem last modified dates of the cached preview and the real preview, and if the cached one is older, remove it and recache it. And yes, I think a clear cache button might be needed for cases when things get out of sync since the application doesn't have exclusive control over the file system and doing a full scan could be slow.

I'd say avoid caching unless after testing there is a performance issue caused by converting between image formats.

[cdb-boop]

Oh, and when thinking about it previously, I think moving model files around also seemed tricky when dealing with cached previews.