Fixed #477 : Corrected state for get and get by id command. #482
Conversation
Codecov Report
@@ Coverage Diff @@
## master #482 +/- ##
==========================================
+ Coverage 32.09% 32.16% +0.06%
==========================================
Files 44 44
Lines 3103 3112 +9
==========================================
+ Hits 996 1001 +5
- Misses 2017 2019 +2
- Partials 90 92 +2
Continue to review full report at Codecov.
|
parauliya
force-pushed
the
Fix_477
branch
2 times, most recently
from
246f699 to
b08bcb1
Compare
parauliya
requested review from
thebsdbox and
jacobweinstock
and removed request for
gianarb
grpc-server/workflow.go
Outdated
| if wfCtx.CurrentActionState != workflow.State_STATE_SUCCESS { | ||
| return wfCtx.CurrentActionState | ||
| } else { | ||
| //wfacts, _ := db.GetWorkflowActions(ctx, id) |
There was a problem hiding this comment.
should probably removed commented code
grpc-server/workflow.go
Outdated
| @@ -279,3 +273,22 @@ func (s *server) ShowWorkflowEvents(req *workflow.GetRequest, stream workflow.Wo | |||
| metrics.CacheHits.With(labels).Inc() | |||
| return nil | |||
| } | |||
|
|
|||
| // This function will provide the workflow state on the basis of the state of the actions | |||
| // For e.g. : If an action has Tailed or Timeout then the workflow state will also be | |||
There was a problem hiding this comment.
NIT: misspelled word "Tailed" -> "Failed"
| @@ -181,6 +174,7 @@ func (s *server) ListWorkflows(_ *workflow.Empty, stream workflow.WorkflowServic | |||
| Hardware: w.Hardware, | |||
| CreatedAt: w.CreatedAt, | |||
| UpdatedAt: w.UpdatedAt, | |||
| State: getWorkflowState(s.db, stream.Context(), w.ID), | |||
There was a problem hiding this comment.
you're making 2 calls to getWorkflowState, this means 2 DB calls. You could call getWorkflowState just once (1 DB call) and store it to a variable for use in both locations.
There was a problem hiding this comment.
I don't think it is possible to call getWorkflowState once and store it because the workflow state will change during the execution of a workflow which makes it dynamic. Therefore we have to fetch the latest value of a workflow state.
There was a problem hiding this comment.
Ah, apologies. You are right. For some reason I thought I saw two calls in a single function.
Signed-off-by: parauliya <[email protected]>
Signed-off-by: parauliya <[email protected]>
Signed-off-by: parauliya <[email protected]>
Signed-off-by: parauliya [email protected]
Description
After this Fix user will get the correct state in
tink workflow getandtink workflow get <id>Why is this needed
Fixes: #477
How Has This Been Tested?
Tested Manually with Vagrant Setup.
How are existing users impacted? What migration steps/scripts do we need?
No Impact.
Checklist:
I have: