Revamp TaskResult of UnifiedAPI #93
Conversation
| Logger?.LogTrace("Response handler for {taskId}", | ||
| resultStatusData.TaskId); | ||
|
|
||
| var result = ProtoSerializer.DeSerializeMessageObjectArray(byteResult); |
There was a problem hiding this comment.
Maybe distinguish between error from deserializing and handler execution ?
| var resultStatusCollection = SessionService.GetResultStatus(bucket); | ||
| if (ResultHandlerDictionary.IsEmpty) | ||
| { | ||
| Thread.Sleep(100); |
# Conflicts: # UnifiedApi/Client/Services/Submitter/Service.cs
lemaitre-aneo
force-pushed
the
fl/revamp-task-result
branch
from
629a508 to
641dc54
Compare
| var resultStatusCollection = SessionService.GetResultStatus(bucket); | ||
| if (ResultHandlerDictionary.IsEmpty) | ||
| { | ||
| Thread.Sleep(100); |
There was a problem hiding this comment.
Use WaitInSeconds Array line 271 rather than a hard coded value. 10 ms is slightly enough
| ResultId = resultStatusData.ResultId, | ||
| Session = SessionId, | ||
| }, | ||
| CancellationToken.None) |
There was a problem hiding this comment.
No usage of CancellationResultTaskSource.Token instead of CancellationToker.None ?
| Logger?.LogError(e2, | ||
| "An exception was thrown while handling a previous exception in the result handler of {taskId}", | ||
| taskId); |
There was a problem hiding this comment.
Re-throw exception here. User process should stop here since they don't manage properly the exception
| ResultHandlerDictionary[resultStatusData.TaskId] | ||
| .HandleError(new ServiceInvocationException(details, | ||
| statusCode), | ||
| resultStatusData.TaskId); |
There was a problem hiding this comment.
Add specific Try catch to manage Handle error ?
| taskStatus = SessionService.GetTaskStatus(resultStatusData.TaskId); | ||
| if (!StatusCodesLookUp.TryGetValue(taskStatus, | ||
| out var statusCode)) | ||
| { | ||
| statusCode = ArmonikStatusCode.Unknown; | ||
| } |
There was a problem hiding this comment.
Add specific exception for GetTaskStatus Grpc Request and call HandleError with Connection error exception if an exception occurs
| Thread.Sleep(waitInSeconds[emptyFetch]); | ||
| emptyFetch = fetched == 0 | ||
| ? Math.Min(emptyFetch + 1, | ||
| waitInSeconds.Length) |
There was a problem hiding this comment.
is it waitInSeconds.Length - 1 instead ?
|
@lemaitre-aneo do we continue or close ? |
I don't know. I still think the change is relevant, but there was a slow down introduced by this change and I don't know where. We could close it and re-open it if we have more insight. |
|
We can wait for the new developments and we can try again |
No description provided.