-
Notifications
You must be signed in to change notification settings - Fork 29
Memory error on customer import #97
Memory error on customer import #97
Conversation
…ustomer data in the database on low memory systems - replace the usage of the CollectionByPagesIteratorFactory with a fetchAll when loading current customers from the database, this was causing issues because of the extra overhead that the the iterator caused by using objects to store each row's data in this way we can use a similar approach to how the product import loads old skus. See: Magento\Catalog\Model\ResourceModel\Product::getProductEntitiesInfo
…t use the DataObject - replace current usage, - keep BC by using the toArray method of the DataObject in the current public method
I have some open questions on this solution I would like to get people's thoughts on. There are two approaches here either replace the usage of The current Storage class allows you to init it with a page size. My current changes ignores this and just loads all the customers. I am not too sure in what cases page size is setup but it maybe that we find some cases that only load a limited set of customers during import or export. |
* @param array $customer | ||
* @return $this | ||
*/ | ||
public function addCustomerByArray(array $customer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not such a big fan of doing things via array. Since we are only using id, email and website id would people suggest passing these in specifically?
Also I am still working on the tests but I thought I would get people's thoughts first. |
- since we are now working with the connection in the load method we need to correct mocks in place
- uses getMockBuilder - only has items that are needed
- make sure that we are testing the new method for adding customer by array
… object is used - in the addCustomer covert $customerData['id'] to $customerData['entity_id'] so that older code that uses customer objects will still work as expected
if (!isset($this->_customerIds[$email])) { | ||
$this->_customerIds[$email] = []; | ||
$customerData = $customer->toArray(); | ||
if (!isset($customerData['entity_id']) && isset($customer['id'])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added this in here as there could be the case that we get an object that only has the getId
but we converted to array that means we have id as the key not the entity_id as we expect. To me this leads me down the route that the new add method should be more like addCustomerByData(int $customerId, int $websiteId, string $email)
this way we would not need such an if here.
…port/CustomerComposite.php - make sure that the data format used when importing data matches the expected format
- fix up the lines of over 120 characters - import classes that will be used to shorten lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The resulting code looks like equivalent to how it worked before with the replacement of collection with the direct select requests.
I noticed only unit tests are added. Can you add integration as well?
Fix Calls to count()
Description
With the following settings a customer import would crash with an out of memory problem.
Fixed Issues (if relevant)
Manual testing scenarios
Contribution checklist