mollie / magento2

Mollie Payments for Magento 2
https://www.mollie.com
Other
104 stars 53 forks source link

First setup:upgrade fails on AddMollieShipmentIdAttribute patch #822

Open LucaGallinari opened 1 month ago

LucaGallinari commented 1 month ago

Describe the bug Sometimes, during the first setup:upgrade of the module, the procedure fails while adding the mollie_shipment_id to the sales_shipment table:

Module 'Hyva_Checkout':
Module 'MarkShust_DisableTwoFactorAuth':
Module 'Mollie_Payment':
Enabling caches:
Current status:
layout: 1
block_html: 1
full_page: 1
Unable to apply data patch Mollie\Payment\Setup\Patch\Data\AddMollieShipmentIdAttribute for module Mollie_Payment. Original exception message: DDL statements are not allowed in transactions
  error  in magento2.php on line 302:
exit code 1 (General error)

Theoretically this should always fails as you shouldn't be able to add a column via DataPatch, but sometimes, especially on the second run of the setup:upgrade, it adds the column properly without errors.

Btw, it should be replaced with a proper definition of the column via db_schema file.

Used versions

Frank-Magmodules commented 1 month ago

Hi @LucaGallinari,

Thank you for opening this issue and providing the details. We’ve had the setup scripts integrated into our Continuous Integration workflow for quite some time now, and we’ve never encountered this problem. Could it be that there is a conflict with another module in your setup?

JeroenBoersma commented 2 weeks ago

I just ran into exactly the same problem.

It only happens the first time you run the script, the second time everything is fine!

Installation: Clean Magento 2 with sample data!

$ composer require mollie/magento2
# default stuff
$ bin/magento setup:upgrade
# ....
Module 'Mollie_Payment':                                                                                 
Unable to apply data patch Mollie\Payment\Setup\Patch\Data\AddMollieShipmentIdAttribute for module Mollie_Payment. Original exception message: DDL statements are not allowed in transactions                      
$ bin/magento setup:upgrade
# OK
Frank-Magmodules commented 2 weeks ago

Thanks @JeroenBoersma , we will take a look into it!

JeroenBoersma commented 1 week ago

So, I can reproduce it time after time...

# install a clean Magento, nothing else and run setup:install
$ rm -rf generated
$ composer require mollie/magento2
$ bin/magento setup:upgrade --keep-generated
# ....
Module 'Mollie_Payment':                                                                                 
Unable to apply data patch Mollie\Payment\Setup\Patch\Data\AddMollieShipmentIdAttribute for module Mollie_Payment. Original exception message: DDL statements are not allowed in transactions                      
$ bin/magento setup:upgrade

So my last comment was incomplete, the --keep-generated messes things up! I know for sure that the flag was there when I commented earlier, but I didn't think it was part of the problem.

@LucaGallinari did you also use the --keep-generated flag? Just to find the common problem, as today I ran multiple scenarios but only this flag made the problem repeatable.

System settings:

Frank-Magmodules commented 1 week ago

That’s helpful, @JeroenBoersma! We spent this morning trying to reproduce it, and now it seems we have a good lead on where and with what setup this might be happening. Thanks—I’ll follow up on this soon!

LucaGallinari commented 1 week ago

@LucaGallinari did you also use the --keep-generated flag? Just to find the common problem, as today I ran multiple scenarios but only this flag made the problem repeatable.

It could be it! Locally i don't use that flag infact i never had this problem, instead on our CI/deploy we are using that flag and it failed on first "runs".

Btw, as i pointed out in the original post, the correct way to add a column is via a Declarative Schema and NOT via a DataPatch. We never run into this kind of problems by using declarative schema.