-
Notifications
You must be signed in to change notification settings - Fork 53
Add Pending State and State Message in Webapi Agent #399
Add Pending State and State Message in Webapi Agent #399
Conversation
Signed-off-by: Future Outlier <[email protected]>
Signed-off-by: Future Outlier <[email protected]>
@pingsutw Just fixed my error, please review! |
Signed-off-by: Future Outlier <[email protected]>
Signed-off-by: Future Outlier <[email protected]>
Please approve it again, thanks a lot! |
Signed-off-by: Future Outlier <[email protected]>
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #399 +/- ##
==========================================
+ Coverage 62.73% 64.23% +1.49%
==========================================
Files 156 156
Lines 13173 10649 -2524
==========================================
- Hits 8264 6840 -1424
+ Misses 4284 3187 -1097
+ Partials 625 622 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Please review again, thanks a lot! |
Signed-off-by: Future Outlier <[email protected]>
Just tested it in Databricks agent service, please review it, thanks a lot! |
@pingsutw / @Future-Outlier can we add more states like initializing etc, all the ones we already have in the task phases? Also have messages for each phase sent by the agent |
@@ -162,6 +162,8 @@ func (p Plugin) Status(ctx context.Context, taskCtx webapi.StatusContext) (phase | |||
taskInfo := &core.TaskInfo{} | |||
|
|||
switch resource.State { | |||
case admin.State_PENDING: | |||
return core.PhaseInfoQueuedWithTaskInfo(core.DefaultPhaseVersion, "Job is PENDING", taskInfo), nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the message. This message does not convey any information to the user. Can the agent send this message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flyteplugins/go/tasks/pluginmachinery/core/phase.go
Lines 220 to 226 in 503d1be
func PhaseInfoInitializing(t time.Time, version uint32, reason string, info *TaskInfo) PhaseInfo { | |
pi := phaseInfo(PhaseInitializing, version, nil, info, false) | |
pi.reason = reason | |
return pi | |
} | |
Is this solution suitable?
Signed-off-by: Future Outlier <[email protected]>
… into pending-state-in-agent
Signed-off-by: Future Outlier <[email protected]>
Signed-off-by: Future Outlier <[email protected]>
Signed-off-by: Future Outlier <[email protected]>
… into pending-state-in-agent
Signed-off-by: Future Outlier <[email protected]>
edit: Re-opening these PRs as the automation failed. |
Hi, we are moving all Flyte development to a monorepo. In order to help the transition period, we're moving open PRs to monorepo automatically and your PR was moved to flyteorg/flyte#4133. Notice that if there are any conflicts in the resulting PR they most likely happen due to the change in the import path of the flyte components. |
TL;DR
As title.
Type
Are all requirements met?
Complete description
For Databricks agent service, we need to have a pending state.
Now we can see what's happening in the pending state!
Follow-up issue
flyteorg/flyteidl#440
flyteorg/flytekit#1834