Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions rest-api/api/pkg/api/handler/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (gth GetTaskHandler) Handle(c echo.Context) error {
return cutil.NewAPIErrorResponse(c, http.StatusNotFound, "Task not found", nil)
}

apiTask := model.NewAPIRackTask(tasks[0])
apiTask := model.NewAPIRackTask(tasks[0], model.WithReport())

logger.Info().Msg("finishing API handler")

Expand Down Expand Up @@ -354,7 +354,7 @@ func (cth CancelTaskHandler) Handle(c echo.Context) error {
return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute Task cancellation workflow on Site: %s", unwrapErr), nil)
}

apiTask := model.NewAPIRackTask(flowResponse.GetTask())
apiTask := model.NewAPIRackTask(flowResponse.GetTask(), model.WithReport())

logger.Info().Msg("finishing API handler")
return c.JSON(http.StatusAccepted, apiTask)
Expand Down Expand Up @@ -393,6 +393,7 @@ func NewGetRackTasksHandler(dbSession *cdb.Session, tc tClient.Client, scp *sc.C
// @Param id path string true "UUID of the Rack"
// @Param siteId query string true "ID of the Site"
// @Param activeOnly query boolean false "Restrict to non-terminal Tasks"
// @Param withReport query boolean false "Include the per-task execution report in each response (default false)"
// @Param pageNumber query integer false "Page number of results returned"
// @Param pageSize query integer false "Number of results per page"
// @Success 200 {array} model.APIRackTask
Expand Down Expand Up @@ -490,6 +491,7 @@ func (h GetRackTasksHandler) Handle(c echo.Context) error {
flowRequest := &flowv1.ListTasksRequest{
RackId: &flowv1.UUID{Id: rackID},
ActiveOnly: apiRequest.ActiveOnly,
WithReport: apiRequest.WithReport,
}
if pageRequest.Offset != nil && pageRequest.Limit != nil {
flowRequest.Pagination = &flowv1.Pagination{
Expand Down Expand Up @@ -527,9 +529,13 @@ func (h GetRackTasksHandler) Handle(c echo.Context) error {
return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute workflow to retrieve all Rack Tasks: %s", unwrapErr), nil)
}

var taskOpts []model.APIRackTaskOption
Comment thread
kunzhao-nv marked this conversation as resolved.
Outdated
if apiRequest.WithReport {
taskOpts = append(taskOpts, model.WithReport())
}
apiTasks := make([]*model.APIRackTask, 0, len(flowResponse.GetTasks()))
for _, t := range flowResponse.GetTasks() {
apiTasks = append(apiTasks, model.NewAPIRackTask(t))
apiTasks = append(apiTasks, model.NewAPIRackTask(t, taskOpts...))
}

total := int(flowResponse.GetTotal())
Expand Down Expand Up @@ -576,6 +582,7 @@ func NewGetTrayTasksHandler(dbSession *cdb.Session, tc tClient.Client, scp *sc.C
// @Param id path string true "UUID of the Tray"
// @Param siteId query string true "ID of the Site"
// @Param activeOnly query boolean false "Restrict to non-terminal Tasks"
// @Param withReport query boolean false "Include the per-task execution report in each response (default false)"
// @Param pageNumber query integer false "Page number of results returned"
// @Param pageSize query integer false "Number of results per page"
// @Success 200 {array} model.APIRackTask
Expand Down Expand Up @@ -673,6 +680,7 @@ func (h GetTrayTasksHandler) Handle(c echo.Context) error {
flowRequest := &flowv1.ListTasksRequest{
ComponentId: &flowv1.UUID{Id: trayID},
ActiveOnly: apiRequest.ActiveOnly,
WithReport: apiRequest.WithReport,
}
if pageRequest.Offset != nil && pageRequest.Limit != nil {
flowRequest.Pagination = &flowv1.Pagination{
Expand Down Expand Up @@ -710,9 +718,13 @@ func (h GetTrayTasksHandler) Handle(c echo.Context) error {
return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute workflow to retrieve all Tray Tasks: %s", unwrapErr), nil)
}

var taskOpts []model.APIRackTaskOption
if apiRequest.WithReport {
taskOpts = append(taskOpts, model.WithReport())
}
apiTasks := make([]*model.APIRackTask, 0, len(flowResponse.GetTasks()))
for _, t := range flowResponse.GetTasks() {
apiTasks = append(apiTasks, model.NewAPIRackTask(t))
apiTasks = append(apiTasks, model.NewAPIRackTask(t, taskOpts...))
}

total := int(flowResponse.GetTotal())
Expand Down
54 changes: 43 additions & 11 deletions rest-api/api/pkg/api/model/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package model

import (
"encoding/json"
"fmt"
"net/url"
"strconv"
Expand All @@ -25,20 +26,41 @@ var ProtoToAPIRackTaskStatusName = map[flowv1.TaskStatus]string{

// APIRackTask is the API response model for a rack task (OpenAPI schema RackTask).
type APIRackTask struct {
ID string `json:"id"`
Status string `json:"status"`
Description string `json:"description"`
Message string `json:"message"`
Started *time.Time `json:"started"`
Finished *time.Time `json:"finished"`
Created time.Time `json:"created"`
Updated time.Time `json:"updated"`
ID string `json:"id"`
Status string `json:"status"`
Description string `json:"description"`
Message string `json:"message"`
Started *time.Time `json:"started"`
Finished *time.Time `json:"finished"`
Created time.Time `json:"created"`
Updated time.Time `json:"updated"`
Report *json.RawMessage `json:"report,omitempty"`
Comment thread
kunzhao-nv marked this conversation as resolved.
Outdated
}

func (t *APIRackTask) FromProto(task *flowv1.Task) {
// APIRackTaskOption configures optional fields populated on an APIRackTask.
// Used by NewAPIRackTask so list endpoints can omit large optional payloads
// (Report in particular) by default while single-task endpoints opt in.
type APIRackTaskOption func(*apiRackTaskOptions)

type apiRackTaskOptions struct {
withReport bool
}

// WithReport populates APIRackTask.Report from Task.report when the proto
// field is non-empty. Without this option, Report is left nil and is
// omitted from the JSON response.
func WithReport() APIRackTaskOption {
return func(o *apiRackTaskOptions) { o.withReport = true }
}

func (t *APIRackTask) FromProto(task *flowv1.Task, opts ...APIRackTaskOption) {
if task == nil {
return
}
o := apiRackTaskOptions{}
for _, opt := range opts {
opt(&o)
}
if task.GetId() != nil {
t.ID = task.GetId().GetId()
}
Expand All @@ -55,11 +77,17 @@ func (t *APIRackTask) FromProto(task *flowv1.Task) {
}
t.Created = task.GetCreatedAt().AsTime().UTC()
t.Updated = task.GetUpdatedAt().AsTime().UTC()
if o.withReport {
if r := task.GetReport(); r != "" {
raw := json.RawMessage(r)
t.Report = &raw
}
}
}

func NewAPIRackTask(task *flowv1.Task) *APIRackTask {
func NewAPIRackTask(task *flowv1.Task, opts ...APIRackTaskOption) *APIRackTask {
t := &APIRackTask{}
t.FromProto(task)
t.FromProto(task, opts...)
return t
}

Expand Down Expand Up @@ -93,6 +121,7 @@ func (r *APICancelTaskRequest) Validate() error {
type APIGetTasksRequest struct {
SiteID string `query:"siteId"`
ActiveOnly bool `query:"activeOnly"`
WithReport bool `query:"withReport"`
Comment thread
kunzhao-nv marked this conversation as resolved.
Outdated
}

func (r *APIGetTasksRequest) Validate() error {
Expand All @@ -111,6 +140,9 @@ func (r *APIGetTasksRequest) QueryValues(page pagination.PageRequest) url.Values
if r.ActiveOnly {
v.Set("activeOnly", strconv.FormatBool(r.ActiveOnly))
}
if r.WithReport {
v.Set("withReport", strconv.FormatBool(r.WithReport))
}
if page.PageNumber != nil && *page.PageNumber != 0 {
v.Set("pageNumber", strconv.Itoa(*page.PageNumber))
}
Expand Down
59 changes: 59 additions & 0 deletions rest-api/api/pkg/api/model/task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
flowv1 "github.com/NVIDIA/infra-controller/rest-api/workflow-schema/flow/protobuf/v1"
"github.com/stretchr/testify/assert"
"google.golang.org/protobuf/types/known/timestamppb"

"github.com/NVIDIA/infra-controller/rest-api/api/pkg/api/pagination"
)

func TestNewAPIRackTask(t *testing.T) {
Expand Down Expand Up @@ -170,6 +172,63 @@ func TestNewAPIRackTask_Timestamps(t *testing.T) {
assert.True(t, result.Finished.Equal(endTime))
}

func TestNewAPIRackTask_Report(t *testing.T) {
t.Run("report omitted by default", func(t *testing.T) {
task := &flowv1.Task{
Id: &flowv1.UUID{Id: "task-rep-1"},
Status: flowv1.TaskStatus_TASK_STATUS_RUNNING,
Report: `{"version":1,"stages":[]}`,
}

result := NewAPIRackTask(task)

assert.Nil(t, result.Report, "Report must default to nil so the JSON field is omitted")
})

t.Run("WithReport populates from non-empty proto report", func(t *testing.T) {
body := `{"version":1,"stages":[{"name":"reset","status":"Succeeded"}]}`
task := &flowv1.Task{
Id: &flowv1.UUID{Id: "task-rep-2"},
Status: flowv1.TaskStatus_TASK_STATUS_RUNNING,
Report: body,
}

result := NewAPIRackTask(task, WithReport())

assert.NotNil(t, result.Report)
assert.JSONEq(t, body, string(*result.Report))
})

t.Run("WithReport on empty proto report still yields nil", func(t *testing.T) {
task := &flowv1.Task{
Id: &flowv1.UUID{Id: "task-rep-3"},
Status: flowv1.TaskStatus_TASK_STATUS_PENDING,
}

result := NewAPIRackTask(task, WithReport())

assert.Nil(t, result.Report, "Empty proto report must not surface as an empty JSON value")
})
}

func TestAPIGetTasksRequest_QueryValues(t *testing.T) {
t.Run("withReport=true surfaces in query values", func(t *testing.T) {
req := APIGetTasksRequest{SiteID: "site-x", WithReport: true}
v := req.QueryValues(pagination.PageRequest{})

assert.Equal(t, "true", v.Get("withReport"))
assert.Equal(t, "site-x", v.Get("siteId"))
})

t.Run("withReport=false is omitted from query values", func(t *testing.T) {
req := APIGetTasksRequest{SiteID: "site-y"}
v := req.QueryValues(pagination.PageRequest{})

assert.Empty(t, v.Get("withReport"))
assert.False(t, v.Has("withReport"), "Default-false withReport must not affect deterministic workflow ID hashing")
})
}

func TestAPIGetTaskRequest_Validate(t *testing.T) {
tests := []struct {
name string
Expand Down
Loading
Loading