[Admin] Bulk errors grid #3654
[Admin] Bulk errors grid #3654
Conversation
PierreGauthier
force-pushed
the
feat-bulk-error-grid
branch
5 times, most recently
from
d85ad2d to
26ea23d
Compare
| /** | ||
| * Returns the time elapsed (in seconds) since its creation after which a non-live index is to be considered ghost. | ||
| * | ||
| * @return int | ||
| */ | ||
| public function getTimeBeforeGhost(): int; |
There was a problem hiding this comment.
I'm not seeing this being used anywhere...
There was a problem hiding this comment.
Sorry, I forgot to remove it.
I was hesitating between two solutions: injecting IndexSetting and IndexSettingHelper in \Smile\ElasticsuiteIndices\Model\IndexStatusProvider, or adding this method to IndexSetting so that only it needs to be injected in IndexStatusProvider. What do you think?
There was a problem hiding this comment.
Yeah, so as seen together, inject indicesConfig in the helper and move the parseIndexName in the helper too as there is also a method doing preg_replace things there.
There was a problem hiding this comment.
So this getTimeBeforeGhost in the IndexSettings model should not be needed for now.
| /** | ||
| * {@inheritDoc} | ||
| */ | ||
| public function getTimeBeforeGhost(): int | ||
| { | ||
| return $this->helper->getTimeBeforeGhost(); | ||
| } |
There was a problem hiding this comment.
I'm not seeing this being used anywhere...
| /** | ||
| * {@inheritDoc} | ||
| */ | ||
| public function parseIndexName(string $indexName): array | ||
| { | ||
| // 1. Remove prefix | ||
| $alias = $this->helper->getIndexAlias(); | ||
| if (!str_starts_with($indexName, "${alias}_")) { | ||
| throw new \LogicException(sprintf('Index %s is not an elasticsuite index.', $indexName)); | ||
| } | ||
| $indexName = str_replace("${alias}_", '', $indexName); |
There was a problem hiding this comment.
I'm not so sure about throwing an exception nor the location of the method...
There was a problem hiding this comment.
Hum yes you're right, I'll update this to keep the original logic that return false for external index
| $indexData = $this->indexSettings->parseIndexName($indexName); | ||
| $indexDate = $indexData['suffix']; |
There was a problem hiding this comment.
You will now throw an exception everytime you have an "external" index on the cluster, won't you ?
There was a problem hiding this comment.
Hum yes you're right, I'll update this to keep the original logic that return false for external index
PierreGauthier
force-pushed
the
feat-bulk-error-grid
branch
from
26ea23d to
5bea174
Compare
PierreGauthier
force-pushed
the
feat-bulk-error-grid
branch
from
5bea174 to
96e5d0f
Compare
| preg_match_all('/{{([\w]*)}}/', $pattern, $matches); | ||
|
|
||
| if (!empty($matches[1])) { | ||
| $format = ''; | ||
| foreach ($matches[1] as $value) { | ||
| $format .= $value; | ||
| } | ||
| $count = strlen($format); | ||
|
|
||
| try { | ||
| $dateString = substr(preg_replace('/[^0-9]/', '', $indexName), -$count); | ||
| $indexName = substr($indexName, 0, -strlen(preg_replace('/{{|}}/', '', $pattern)) - 1); | ||
|
|
||
| // Tracking indices are built monthly and does not fit with standard pattern containing datetime with hours. | ||
| $datetime = $isTrackingIndex ? false : new \Zend_Date($dateString, $format); | ||
| } catch (\Zend_Date_Exception $e) { | ||
| $datetime = false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Please comment the steps with that they are supposed to do so this is "debugging after a way too heavy meal" proof :)
| */ | ||
| public function recordError(string $index, string $errorCode, string $reason, string $sampleIds): void | ||
| { | ||
| $data = $this->indexSettings->parseIndexName($index); |
There was a problem hiding this comment.
Either here or in the parseIndexName method, I would put in a place a memory cache.
Since it's a big expected that on a bulk data, 99.99% of the time, the data will target the same index.
So if there is a recurring error (due, for instance, to a badly create attribute or data source), we won't spend too much time in parseIndexName which does a log of preg things...
| $datetime = $isTrackingIndex ? false : new \Zend_Date($dateString, $format); | ||
| } catch (\Zend_Date_Exception $e) { | ||
| $datetime = false; |
There was a problem hiding this comment.
If you can, get rid of \Zend_Date and use DateTime as we had to do on 2.11.x.
Otherwise we will have to a "fixing PR" once that branch as been merged on 2.10.x and 2.10.x on 2.11.x
| 'prefix' => 'magento2', | ||
| 'store_code' => 'default', | ||
| 'index_identifier' => 'catalog_product', | ||
| 'datetime' => new \Zend_Date('20250707093823', 'YYYYMMddHHmmss'), |
There was a problem hiding this comment.
(Same as before and for all the following usage)
If you can, get rid of \Zend_Date and use DateTime as we had to do on 2.11.x.
Otherwise we will have to a "fixing PR" once that branch as been merged on 2.10.x and 2.10.x on 2.11.x
No description provided.