Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Add deckURI to NodeExecutionData #413

Merged
merged 18 commits into from
Jun 7, 2022
31 changes: 31 additions & 0 deletions dataproxy/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,37 @@ func (s Service) CreateUploadLocation(ctx context.Context, req *service.CreateUp
}, nil
}

// CreateDownloadLocation creates a temporary signed url to allow callers to download content.
func (s Service) CreateDownloadLocation(ctx context.Context, req *service.CreateDownloadLocationRequest) (
*service.CreateDownloadLocationResponse, error) {
if expiresIn := req.ExpiresIn; expiresIn != nil {
if !expiresIn.IsValid() {
return nil, fmt.Errorf("expiresIn [%v] is invalid", expiresIn)
}

if expiresIn.AsDuration() > s.cfg.Upload.MaxExpiresIn.Duration {
return nil, fmt.Errorf("expiresIn [%v] cannot exceed max allowed expiration [%v]",
expiresIn.AsDuration().String(), s.cfg.Upload.MaxExpiresIn.String())
}
} else {
req.ExpiresIn = durationpb.New(s.cfg.Upload.MaxExpiresIn.Duration)
}

resp, err := s.dataStore.CreateSignedURL(ctx, storage.DataReference(req.NativeUrl), storage.SignedURLProperties{
pingsutw marked this conversation as resolved.
Show resolved Hide resolved
Scope: stow.ClientMethodGet,
ExpiresIn: req.ExpiresIn.AsDuration(),
})

if err != nil {
return nil, fmt.Errorf("failed to create a signed url. Error: %w", err)
}

return &service.CreateDownloadLocationResponse{
SignedUrl: resp.URL.String(),
ExpiresAt: timestamppb.New(time.Now().Add(req.ExpiresIn.AsDuration())),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this valid that we can accurately say here . The library sets this time for signed url if its google
https://github.com/flyteorg/stow/blob/9c3f5f9ea24966206f0b2edc608da6ca803420e3/google/container.go#L55

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not accurate, but I think it's okay for now because we can't get the expiry time from SignedURLResponse
I can add it in a separate PR.

}, nil
}

// createShardedStorageLocation creates a location in storage destination to maximize read/write performance in most
// block stores. The final location should look something like: s3://<my bucket>/<shard length>/<file name>
func createShardedStorageLocation(ctx context.Context, shardSelector ioutils.ShardSelector, store *storage.DataStore,
Expand Down
23 changes: 23 additions & 0 deletions dataproxy/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,26 @@ func TestCreateUploadLocation(t *testing.T) {
assert.Error(t, err)
})
}

func TestCreateDownloadLocation(t *testing.T) {
dataStore, err := storage.NewDataStore(&storage.Config{Type: storage.TypeMemory}, promutils.NewTestScope())
assert.NoError(t, err)
s, err := NewService(config.DataProxyConfig{}, dataStore)
assert.NoError(t, err)

t.Run("Invalid expiry", func(t *testing.T) {
_, err = s.CreateDownloadLocation(context.Background(), &service.CreateDownloadLocationRequest{
NativeUrl: "s3://bucket/key",
ExpiresIn: durationpb.New(-time.Hour),
})
assert.Error(t, err)
})

t.Run("valid config", func(t *testing.T) {
_, err = s.CreateDownloadLocation(context.Background(), &service.CreateDownloadLocationRequest{
NativeUrl: "s3://bucket/key",
ExpiresIn: durationpb.New(time.Hour),
})
assert.Error(t, err)
})
}
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -200,3 +200,5 @@ require (
)

replace github.com/robfig/cron/v3 => github.com/unionai/cron/v3 v3.0.2-0.20210825070134-bfc34418fe84

replace github.com/flyteorg/flyteidl => github.com/flyteorg/flyteidl v1.1.2-0.20220523141814-4fbb9bb1c89e
pingsutw marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 2 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -379,9 +379,8 @@ github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga
github.com/fatih/structtag v1.2.0/go.mod h1:mBJUNpUnHmRKrKlQQlmCrh5PuhftFbNv8Ys4/aAZl94=
github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8SPQ=
github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
github.com/flyteorg/flyteidl v1.0.0/go.mod h1:JW0z1ZaHS9zWvDAwSMIyGhsf+V4zrzBBgh5IuqzMFCM=
github.com/flyteorg/flyteidl v1.1.0 h1:f8tdMXOuorS/d+4Ut2QarfDbdCOriK0S+EnlQzrwz9E=
github.com/flyteorg/flyteidl v1.1.0/go.mod h1:JW0z1ZaHS9zWvDAwSMIyGhsf+V4zrzBBgh5IuqzMFCM=
github.com/flyteorg/flyteidl v1.1.2-0.20220523141814-4fbb9bb1c89e h1:PketgAetY6gDCE6XmXEAYVvlo2XJ8BjrJZznneQfVXA=
github.com/flyteorg/flyteidl v1.1.2-0.20220523141814-4fbb9bb1c89e/go.mod h1:f1tvw5CDjqmrzNxKpRYr6BdAhHL8f7Wp1Duxl0ZOV4g=
github.com/flyteorg/flyteplugins v1.0.0 h1:77hUJjiIxBmQ9rd3+cXjSGnzOVAFrSzCd59aIaYFB/8=
github.com/flyteorg/flyteplugins v1.0.0/go.mod h1:4Cpn+9RfanIieTTh2XsuL6zPYXtsR5UDe8YaEmXONT4=
github.com/flyteorg/flytepropeller v1.1.1 h1:z9OFS7VAsoFOyIGSfIszaMrERG8MOvS17yzpuiusb64=
Expand Down
5 changes: 5 additions & 0 deletions pkg/manager/impl/node_execution_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -507,11 +507,16 @@ func (m *NodeExecutionManager) GetNodeExecutionData(
return nil, err
}

if err != nil {
return nil, err
}

response := &admin.NodeExecutionGetDataResponse{
Inputs: inputURLBlob,
Outputs: outputURLBlob,
FullInputs: inputs,
FullOutputs: outputs,
DeckUri: nodeExecution.Closure.DeckUri,
}

if len(nodeExecutionModel.DynamicWorkflowRemoteClosureReference) > 0 {
Expand Down
10 changes: 7 additions & 3 deletions pkg/manager/impl/node_execution_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"testing"
"time"

"github.com/flyteorg/flyteadmin/pkg/manager/impl/util"

genModel "github.com/flyteorg/flyteadmin/pkg/repositories/gen/models"

eventWriterMocks "github.com/flyteorg/flyteadmin/pkg/async/events/mocks"
Expand Down Expand Up @@ -1192,8 +1194,9 @@ func TestGetNodeExecutionData(t *testing.T) {
expectedClosure := admin.NodeExecutionClosure{
Phase: core.NodeExecution_SUCCEEDED,
OutputResult: &admin.NodeExecutionClosure_OutputUri{
OutputUri: "output uri",
OutputUri: util.OutputsFile,
},
DeckUri: util.DeckFile,
}
dynamicWorkflowClosureRef := "s3://my-s3-bucket/foo/bar/dynamic.pb"

Expand Down Expand Up @@ -1233,7 +1236,7 @@ func TestGetNodeExecutionData(t *testing.T) {
Url: "inputs",
Bytes: 100,
}, nil
} else if uri == "output uri" {
} else if uri == util.OutputsFile {
return admin.UrlBlob{
Url: "outputs",
Bytes: 200,
Expand All @@ -1260,7 +1263,7 @@ func TestGetNodeExecutionData(t *testing.T) {
marshalled, _ := proto.Marshal(fullInputs)
_ = proto.Unmarshal(marshalled, msg)
return nil
} else if reference.String() == "output uri" {
} else if reference.String() == util.OutputsFile {
marshalled, _ := proto.Marshal(fullOutputs)
_ = proto.Unmarshal(marshalled, msg)
return nil
Expand All @@ -1285,6 +1288,7 @@ func TestGetNodeExecutionData(t *testing.T) {
Url: "outputs",
Bytes: 200,
},
DeckUri: util.DeckFile,
FullInputs: fullInputs,
FullOutputs: fullOutputs,
DynamicWorkflow: &admin.DynamicWorkflowNodeMetadata{
Expand Down
5 changes: 5 additions & 0 deletions pkg/manager/impl/util/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ import (
"github.com/golang/protobuf/proto"
)

const (
OutputsFile = "outputs.pb"
DeckFile = "deck.html"
)

func shouldFetchData(config *runtimeInterfaces.RemoteDataConfig, urlBlob admin.UrlBlob) bool {
return config.Scheme == common.Local || config.Scheme == common.None || config.MaxSizeInBytes == 0 ||
urlBlob.Bytes < config.MaxSizeInBytes
Expand Down
1 change: 0 additions & 1 deletion pkg/manager/impl/util/data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ func TestGetInputs(t *testing.T) {
assert.True(t, proto.Equal(fullInputs, testLiteralMap))
assert.Empty(t, inputURLBlob)
})

}

func TestGetOutputs(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions pkg/repositories/transformers/node_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ func addTerminalState(
nodeExecutionModel.ErrorKind = &k
nodeExecutionModel.ErrorCode = &request.Event.GetError().Code
}
closure.DeckUri = request.Event.DeckUri

return nil
}

Expand Down