SCMOD-12730: Initial implementation #1
Conversation
.../src/main/java/com/microfocus/caf/worker/taskunstowing/database/HikariDataSourceFactory.java
Outdated
Show resolved
Hide resolved
| <dependency> | ||
| <groupId>com.github.cafdataprocessing</groupId> | ||
| <artifactId>worker-document-shared</artifactId> | ||
| </dependency> |
There was a problem hiding this comment.
Is worker-document-shared directly required?
There was a problem hiding this comment.
No, removed now
| <dependency> | ||
| <groupId>org.jdbi</groupId> | ||
| <artifactId>jdbi3-sqlobject</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.jdbi</groupId> | ||
| <artifactId>jdbi3-postgres</artifactId> | ||
| </dependency> |
There was a problem hiding this comment.
Should these be declared test scope?
There was a problem hiding this comment.
One can be test scope, the other needs to be runtime scoped, updated now.
| stowedTaskRow.getTaskData(), | ||
| TaskStatus.valueOf(stowedTaskRow.getTaskStatus()), | ||
| stowedTaskRow.getContext() != null | ||
| ? OBJECT_MAPPER.readValue(stowedTaskRow.getContext(), Map.class) |
There was a problem hiding this comment.
This deserializes to a Map<String, byte[]> correctly even though it was only told to deserialize to a Map?
There was a problem hiding this comment.
Thanks, I've updated this to use a TypeReference now to specify that the type should be Map<String, byte[]>
There was a problem hiding this comment.
Thanks, I've updated this to use a TypeReference now to specify that the type should be Map<String, byte[]>
I need to double-check and test this a bit more, I'm not entirely sure I'm serializing and deserializing the context properly. Do you have any examples of actual context objects that are used? I've only ever seen context being null or an empty map in my testing.
There was a problem hiding this comment.
I think the problem here is this:
- A task message is sent to any worker with a context that looks like this:
"context": {
"caf/worker": "ew0KICAia2V5MSI6ICJ2YWx1ZTEiDQp9"
},
The key must be caf/worker (or whatever the service path is for that particular worker) otherwise won't get passed to the worker.
The value is a base64 encoded map:
- The worker framework uses the
caf/workerkey to pull the value out of thisMap<String, byte[]>, passing on just thebyte[]to the worker:
- The Task Stowing Worker stores this
byte[]in the database:
- The Task Unstowing Worker reads this
byte[]from the database, however, the Task Unstowing Worker actually needs aMap<String, byte[]>in order to create a TaskMessage to sent onto the intended worker:
So basically, I think the Task Stowing Worker needs to know the servicePath key for the worker that sent it the message to be stowed, and would also need a new db column to store it maybe - as the Task Unstowing Worker will need to know the servicePath in order to reconstruct the context for the TaskMessage, I'm just not sure the best way to go about it? The ServicePath is currently not exposed anywhere as far as I can see:
My initial thoughts were that when a worker sends a task message to the paused queue, it would have to include its servicePath in the taskData somehow, maybe as custom data, but that feels like it's getting a bit messy?
There was a problem hiding this comment.
Removed context as discussed
...unstowing/src/main/java/com/github/jobservice/workers/taskunstowing/TaskUnstowingWorker.java
Outdated
Show resolved
Hide resolved
| processFailure(failure, exception, document); | ||
| } | ||
| } | ||
| } catch (final Exception exception) { |
There was a problem hiding this comment.
What exception is this trying to catch here? As is it this defeats the transient issue handling above by catching the DocumentWorkerTransientException and turning it into a non-transient failure. I suspect the catch block shouldn't be present at all.
There was a problem hiding this comment.
I'm catching the exception that might be thrown when trying to send a message to rabbit, but its in the wrong place, I've moved it now.
...unstowing/src/main/java/com/github/jobservice/workers/taskunstowing/TaskUnstowingWorker.java
Outdated
Show resolved
Hide resolved
...ng/src/main/java/com/github/jobservice/workers/taskunstowing/TaskUnstowingWorkerFailure.java
Outdated
Show resolved
Hide resolved
| final int numStowedTaskRowsBeforeExcluding = stowedTaskRows.size(); | ||
| stowedTaskRows = stowedTaskRows.stream() | ||
| .filter(stowedTask -> !stowedTaskRowsToExclude.contains(stowedTask)) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
This may have been more efficient in SQL but I couldn't figure out exactly how to do it. I think it's probably ok as this is an error case scenario and hopefully unlikely to happen often.
https://portal.digitalsafe.net/browse/SCMOD-12730