Feature/add ingestion cron #36
Conversation
|
Getting a bit nuts about this condition coverage. |
...main/java/eu/dissco/orchestration/backend/controller/MachineAnnotationServiceController.java
Outdated
Show resolved
Hide resolved
src/main/java/eu/dissco/orchestration/backend/domain/SourceSystem.java
Outdated
Show resolved
Hide resolved
src/main/java/eu/dissco/orchestration/backend/service/SourceSystemService.java
Show resolved
Hide resolved
| } | ||
|
|
||
| private static String generateJobName(SourceSystemRecord sourceSystem, boolean isCron) { | ||
| var name = sourceSystem.sourceSystem().translatorType().name().toLowerCase() + "-" + getSuffix( |
There was a problem hiding this comment.
I can't find a name() function in translatorType. The function getName() returns the constant "translator_type". Is it possible you want to use translatorType().getLiteral() instead?
There was a problem hiding this comment.
The name function is available by default for enums. It just returns the name of the enum as a string. So in this case it will be the same as the literal. Changed it to the literal, as that might be better understandable.
There was a problem hiding this comment.
didn't know that about enums! Either function should be good
src/main/java/eu/dissco/orchestration/backend/service/SourceSystemService.java
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| private void createTranslatorJob(SourceSystemRecord sourceSystemRecord, boolean rollback) |
There was a problem hiding this comment.
Just semantics, but calling this argument "rollback" says to me, "rollback whatever was created". Maybe call it "rollbackOnFailure" or something a bit more clear
src/main/java/eu/dissco/orchestration/backend/service/SourceSystemService.java
Show resolved
Hide resolved
| @@ -98,12 +193,35 @@ public JsonApiWrapper updateSourceSystem(String id, SourceSystem sourceSystem, S | |||
| var sourceSystemRecord = new SourceSystemRecord(id, currentSourceSystem.version() + 1, userId, | |||
| Instant.now(), null, sourceSystem); | |||
| repository.updateSourceSystem(sourceSystemRecord); | |||
| updateCronJob(sourceSystemRecord, currentSourceSystem); | |||
There was a problem hiding this comment.
Should we also re-run ingestion, like we do on creation of a source system?
There was a problem hiding this comment.
Fair point, I was doubting about this. However, when you do a minor change in, I don't know the description, it might not be necessary. You can now always trigger it separately as well through the run endpoint.
There was a problem hiding this comment.
Can we include this in the request? A user can check a box that says "re-run ingestion on update"? Then pass that boolean to this function.
Can be a separate user story.
There was a problem hiding this comment.
💯 But in a new user story https://naturalis.atlassian.net/browse/DD-1031
src/main/java/eu/dissco/orchestration/backend/service/SourceSystemService.java
Show resolved
Hide resolved
|
Added functionality to the source system endpoint.
When a new SourceSystem is added:
On update:
On delete:
Additional endpoint to schedule an extra run of the job.
Upgraded kubernetes library which impacted MAS code.
Large update of all libs.
Removed old/deprecated translator code.