product_image_filestore: when updating module it throws error. about misc-addons HOT 9 CLOSED

I guess, image column is restored after updating product module, which leads to error.

@jrial, as a author of that updates could assist on fixing this, please?

Probably, we need to use alternative check before renaming the column.

Current check is:

    cr.execute("select COUNT(*) from information_schema.columns where table_name='product_template' AND column_name='image';")
    res = cr.dictfetchone()
    if res.get('count'):
        _logger.info('Starting conversion for product_template: saving data for further processing.')
        # Rename image column so we don't lose images upon module install
        cr.execute("ALTER TABLE product_template RENAME COLUMN image TO image_old")
        cr.commit()
    else:
        _logger.info('No image field found in product_template; no data to save.')

from misc-addons.

I no longer work with Odoo, but I'll give it a shot.

First some explanation on how the product_image_filestore auto-upgrade works:

1. The image column, if present, gets renamed to image_old. This happens in _auto_init, before the new fields are added.
2. The module installs normally and updates its columns/fields.
3. The image_old column, if present, is rewritten to the filestore. The field then gets overwritten with null in the DB to free up the space. This happens in _auto_end.
4. Afterwards, still in _auto_end, the column image_old gets renamed to image_bkp so that subsequent upgrades of this module don't bother with trying to rewrite its contents to the filestore.

From this, I can already guess what went wrong: every time you upgrade everything, you reinstate the "image" column when the "product" module where it's defined gets updated. The timeline is as follows:

1. The first installation of product_image_filestore removes image, and replaces it with image_bkp.
2. The -u all reinstates image, which gets renamed to image_old. But when image_old should get renamed to image_bkp, it fails because that column already exists. At that point, image has disappeared, image_old was added, and image_bkp is still there from the previous upgrade.
3. All upgrades after this will encounter an image_old column, and fail to rename image. This is the error you pasted, but it's not the first error you encountered.

@yelizariev : The code fix: there is no reason to rename image_old to image_bkp if all went well during image conversion. After the for-loop, check if image_old contains data. If it does, rename to _bkp so the data doesn't get lost. But if it doesn't, simply drop image_old instead of renaming it. Needs to be done for both models.

@aaltinisik : The fix for your database: check if there's anything in the image_bkp and image_old columns. If not, simply drop them. And word of advice: "-u all" is a recipe for disaster, although that doesn't excuse my code for breaking, of course. ;)

I could create a pull request if you prefer me to do it, but it'll take a while as I'm currently rather busy.

from misc-addons.

@jrial : "-u all" is for update and module should cope with that.
I think code that moves pictures from DB to filestore should be seperate and run with a wizard or from cron job and should convert just a limited number of images (to cope with timeout)

from misc-addons.

I know; I'm just informing you it's a recipe for disaster. Worked with Odoo for 4 years, of which 3.5 at the company itself, and I can no longer count the number of development databases that went broken beyond repair due to weird interactions between modules when updating all.

I'll fix my code because I don't want to contribute to the problem. But I'm advising you it's a better idea to update only the modules that have changed whenever possible.

from misc-addons.

@jrial thank you for explanation.
Currently, I am focused on 9.0 version, so your help are very welcome

from misc-addons.

Here you go. Tested on a fresh DB by repeatedly restarting with -u all. Verified that it preserves the state of the images that were altered/dropped before installing the module.

When the upgrade fails, the old columns are preserved with the _bkp suffix in order to preserve the unconverted data. When the upgrade succeeds, these old image columns are simply discarded and won't get in the way during subsequent updates of the product module.

from misc-addons.

@aaltinisik I hope the issue is resolved for you

from misc-addons.

will test and report

from misc-addons.

@aaltinisik : if you still have lingering _old and _bkp fields in your DB, make sure they're empty and toss them out before testing. The fix will prevent similar issues from cropping up in the future, but it won't fix your current DB.

In fact, I should probably add another check in the __auto_begin method to see if there's a _bkp column, and abort the upgrade in such a case. @yelizariev : I'll probably do another PR soon adding this check as well.

from misc-addons.