Skip to content

feat: retry validation feature (MAPCO-8327)#42

Merged
razbroc merged 68 commits intoalphafrom
feat/retry-api-v2
Dec 11, 2025
Merged

feat: retry validation feature (MAPCO-8327)#42
razbroc merged 68 commits intoalphafrom
feat/retry-api-v2

Conversation

@razbroc
Copy link
Copy Markdown
Contributor

@razbroc razbroc commented Nov 16, 2025

Add new “retry” api for an active failed job caused by “failed” validations task (in case of errors in validations):

First design the new api via swagger.

check if job is “retry-able”:

ingestion job status is “Failed” & validations task status is “Completed”/”Failed” - (Recoveable):

if validation task has been completed with errors → can be try.

if validation task has been completed without errors → cannot be retry

if validation task has been failed:

delete by the polygon-parts-manager api - the validation table → can be retry

use the shp hash for job parameter to compare and detect for a shp file changes (via task parameters):

if equal → throw an Bad Request error with relevant error message

add new checksum (hash) to the task parameters checksums array.

“reset” task validation result within task parameters

set the job and task statuses back to “Pending” for re-consume.

** if the requested retry job is contain more than just “validation” task (means validation completed successfully) - trigger the job-manager api to retry job.

Question Answer
Bug fix
New feature
Breaking change
Deprecations
Documentation
Tests added
Chore

Related issues: #MAPCO-8327

@razbroc razbroc changed the title feat: retry validation feature (merged) feat: retry validation feature (MAPCO-8327) Nov 17, 2025
Comment thread openapi3.yaml Outdated
Comment thread openapi3.yaml Outdated
operationId: retryLayer
tags:
- ingestion
summary: retry a failed ingestion job caused by validation errors
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this retry will handle also other task failures but not only validation errors

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sababa

Comment thread openapi3.yaml
Comment thread openapi3.yaml Outdated
Comment thread openapi3.yaml Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread openapi3.yaml Outdated
Comment thread src/ingestion/interfaces.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated

return validationTask.parameters.isValid
? this.handleRetryWithoutErrors(jobId, validationTask.id, logCtx)
: this.handleRetryWithErrors(jobId, retryJob, validationTask, logCtx);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing await

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why my friend? i return a promise therefor returning an awaited promise is not allowed in this context

Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread helm/values.yaml
Comment thread config/custom-environment-variables.json
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/ingestion/models/ingestionManager.ts Outdated
Comment thread src/serviceClients/polygonPartsManagerClient.ts Outdated
Comment thread tests/unit/ingestion/models/ingestionManager.spec.ts
Comment thread tests/unit/ingestion/models/ingestionManager.spec.ts Outdated
Comment thread tests/unit/ingestion/models/ingestionManager.spec.ts Outdated
Comment thread tests/unit/ingestion/models/ingestionManager.spec.ts
Comment thread tests/unit/ingestion/models/ingestionManager.spec.ts
Comment thread tests/unit/ingestion/models/ingestionManager.spec.ts
Comment thread tests/unit/ingestion/models/ingestionManager.spec.ts
Copy link
Copy Markdown
Contributor

@CL-SHLOMIKONCHA CL-SHLOMIKONCHA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just add what we discussed about:
expect for update task parameters body

razbroc and others added 3 commits December 11, 2025 15:24
* test: add retry ingestion corresponding tests

* style: adjust formatting in IngestionRequestSender constructor and update bad request test cases for ingestion

* style: improve formatting in IngestionRequestSender constructor and update test case descriptions for clarity

* test: add polygonPartsManagerURL to default test configuration

* style:lint

* fix: handle NotFoundError and improve input file path handling in ingestion process + fix tests

* test: update job reset descriptions for clarity in ingestion tests

* test: refactor bad request test cases for ingestion API to improve clarity and structure

* style: lint

* chore: revet coverage to original

* test: update coverage threshold for statements in jest configuration and add checksum unit test

* test: update jest configuration for unused schema and adjust coverage statements

* style: lint

* feat: changed all shapefiles path inside parameters to be relative

* chore: upgrade raster shared (#49)

* chore: update to new raster-shared

* refactor: remove unused validationTaskParametersSchema and update validation logic to use ingestionValidationTaskParamsSchema

* refactor: remove unused checksumSchema import from interfaces

* feat: add generateFullChecksum function and update usages in IngestionManager tests

* Refactor ingestion tests to improve validation cases and adjust mock data generation

- Updated bad request test cases in ingestion.spec.ts to ensure comprehensive coverage of input validation scenarios.
- Adjusted the count of regions in metadata generation to a maximum of 5 instead of 100 for better test performance.
- Modified input files generation to limit gpkgFilesPath to a single file for more controlled testing.
- Ensured consistency in file path generation across various mock functions.

* style: lint

* feat: remove unused generateChecksum import in ingestionManager tests

* refactor: rename getFilesChecksum to getChecksum and update usages

* refactor: update validation task parameters to use IngestionValidationTaskParams and remove unused imports

* refactor: remove trailing whitespace in interfaces.ts

* test: fix pr comments - update error handling and checksum generation in tests

* test: remove excessive timeout from nonexistent file response test

* test: update error handling to throw NotFoundError instead of FileNotFoundError

* fix: update coverage threshold for statements in jest config

---------

Co-authored-by: shlomiko <shlomiko@rafael.co.il>
@razbroc razbroc merged commit 314fb69 into alpha Dec 11, 2025
7 checks passed
@razbroc razbroc deleted the feat/retry-api-v2 branch December 11, 2025 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants