On client error throw, catch and handle exception #38
Conversation
bartlomiejmarszal
requested review from
boajer,
gabrielfs7,
mike-ionut-mihai-sandu-tao and
shpran
There was a problem hiding this comment.
There are some minor comments.
Good job!
- New code is covered by tests (if applicable)
- Tests are running successfully (old and new ones) on my local machine (if applicable)
- New code is respecting code style rules
- New code is respecting best practices
- New code is not subject to concurrency issues (if applicable)
- Feature is working correctly on my local machine (if applicable)
- Acceptance criteria are respected
- Pull request title and description are meaningful
Rename checkErrors
|
This PR is depending to oat-sa/tao-core#2724 |
Separate errors with semicolon.
| $this->sut->buildIndex($iterator); | ||
| } | ||
|
|
||
| private function getMultipleDocs(MockObject $doc, $quantity) |
There was a problem hiding this comment.
I approved the PR, but we could avoid that by making the INDEXING_BLOCK_SIZE injectable / customizable, so yuo do not need to iterate 100 results to test. I think this test will be slow at some point if we do not do it.
Thanks!
There was a problem hiding this comment.
Can you please move the private method as the last? It is in our CR guideline
| ->method('bulk') | ||
| ->willReturn([ | ||
| 'errors' => true, | ||
| 'items' => [ |
There was a problem hiding this comment.
are you sure about the structure of the $clientResponse['items'] ?
|
|
||
| foreach ($clientResponse['items'] as $response) { | ||
| $response = reset($response); | ||
| $errors .= $response['error']['reason'] . '; '; |
There was a problem hiding this comment.
are we using this reason inside FE , did we communicate with them about the glue used?
|
|
||
| foreach ($clientResponse['items'] as $response) { | ||
| $response = reset($response); | ||
| $errors .= $response['error']['reason'] . '; '; |
There was a problem hiding this comment.
as we discussed , let's add a identifier to the processed data so we are able to identify the failing items
also let's check what items are failing
There was a problem hiding this comment.
Well done 👍🏻
- New code is covered by tests (if applicable)
- Tests are running successfully (old and new ones) on my local machine (if applicable)
- New code is respecting code style rules
- New code is respecting best practices
- New code is not subject to concurrency issues (if applicable)
- Feature is working correctly on my local machine (if applicable)
- Acceptance criteria are respected
- Pull request title and description are meaningful
Regarding issue below:
https://oat-sa.atlassian.net/l/c/Zxcvk6W2
I'm introducing ES client operation in try-catch blocks in order to handle errors that may appear from the client.