search

bigbridge-nl / product-import

Fast product import library for Magento 2
MIT License
113 stars 32 forks source link

Module shouldn't rely on magento2-base package to exist #45

Closed hostep closed 3 years ago

hostep commented 3 years ago

Hi guys!

As a best practise, we delete the vendor/magento/magento2-base module when deploying a Magento shop to a server. The code in this module is not used when running a Magento shop, its only purpose is to copy files inside it to the root of the project using a composer plugin. But when that is finished, it serves no longer a purpose anymore. So we remove this module always in order to save disk space.

However, it looks like your module relies on the composer.json file to exist in vendor/magento/magento2-base to figure out the current Magento version, this is done over here: https://github.com/bigbridge-nl/product-import/blob/9420451d5d2cdff1e4fc7139dedbeaeb5d871b3b/Model/Resource/MetaData.php#L422

Is there a chance you could try to revert back to using $productMetadata->getVersion() as mentioned in the comments. It is slower indeed, but Magento seems to have fixed some slowness that was reported. This should be faster now in Magento 2.4.0 and higher by https://github.com/magento/magento2/pull/24030 and https://github.com/magento/magento2/pull/26001.

Would you consider trying to remove the reliance on the vendor/magento/magento2-base module from this module? Or maybe add a fallback for when that vendor/magento/magento2-base/composer.json file is not found to still use $productMetadata->getVersion()? That would be appreciated 🙂

Thanks!

patrick-bigbridge commented 3 years ago

Hello @hostep

That's an interesting note you make there about magento2-base. I had not heard that before.

Since speed is essential to the module, I am not happy with anything that slows it down. But providing a fallback is perfectly acceptable, of course. I will add it somewhere in the next few weeks.

patrick-bigbridge commented 3 years ago

Version 1.7.2 now contains the fallback.

hostep commented 3 years ago

Awesome, thanks for the quick fix!