Take literal hash into account during cache key calculation (flyteorg…

…#406) * Point to flyteidl branch Signed-off-by: Eduardo Apolinario <[email protected]> * Set tag Signed-off-by: Eduardo Apolinario <[email protected]> * Add replaceHashInLiteral function Signed-off-by: Eduardo Apolinario <[email protected]> * Changes to propeller-config.yaml to run locally Signed-off-by: Eduardo Apolinario <[email protected]> * Pick up latest flyteidl@add-hash-to-literal Signed-off-by: Eduardo Apolinario <[email protected]> * Rename variables Signed-off-by: Eduardo Apolinario <[email protected]> * Add unit tests Signed-off-by: Eduardo Apolinario <[email protected]> * s/replaceHashInLiteral/hashify/g Signed-off-by: Eduardo Apolinario <[email protected]> * s/literalMapCopy/hashifiedLiteralMap/g Signed-off-by: Eduardo Apolinario <[email protected]> * Bump flyteidl Signed-off-by: Eduardo Apolinario <[email protected]> * Use flyteidl 0.23.0 Signed-off-by: Eduardo Apolinario <[email protected]> * Revert "Changes to propeller-config.yaml to run locally" This reverts commit 983ee6c580b6b98a2150de8ac1a1f272367659bd. Signed-off-by: Eduardo Apolinario <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]>
eapolinario · Mar 3, 2022 · 4d1a371 · 4d1a371
1 parent fcddb74
commit 4d1a371
Show file tree

Hide file tree

Showing 4 changed files with 618 additions and 2 deletions.
diff --git a/go.mod b/go.mod
@@ -6,7 +6,7 @@ require (
 	github.com/DiSiqueira/GoTree v1.0.1-0.20180907134536-53a8e837f295
 	github.com/benlaurie/objecthash v0.0.0-20180202135721-d1e3d6079fc1
 	github.com/fatih/color v1.10.0
-	github.com/flyteorg/flyteidl v0.22.0
+	github.com/flyteorg/flyteidl v0.23.0
 	github.com/flyteorg/flyteplugins v0.10.9
 	github.com/flyteorg/flytestdlib v0.4.12
 	github.com/ghodss/yaml v1.0.0

diff --git a/go.sum b/go.sum
@@ -239,6 +239,12 @@ github.com/fatih/structtag v1.2.0/go.mod h1:mBJUNpUnHmRKrKlQQlmCrh5PuhftFbNv8Ys4
 github.com/flyteorg/flyteidl v0.21.23/go.mod h1:576W2ViEyjTpT+kEVHAGbrTP3HARNUZ/eCwrNPmdx9U=
 github.com/flyteorg/flyteidl v0.22.0 h1:lVAXyacTCIX6Fl0qWoFzyto9Hfx0ADlyPhHPmOMiuIY=
 github.com/flyteorg/flyteidl v0.22.0/go.mod h1:576W2ViEyjTpT+kEVHAGbrTP3HARNUZ/eCwrNPmdx9U=
+github.com/flyteorg/flyteidl v0.22.2-0.20220217205210-d817026005f3 h1:b/mCyBe9+Yw1LTBJRRXtsZO6TH6TqzzKaup8gkhus2U=
+github.com/flyteorg/flyteidl v0.22.2-0.20220217205210-d817026005f3/go.mod h1:576W2ViEyjTpT+kEVHAGbrTP3HARNUZ/eCwrNPmdx9U=
+github.com/flyteorg/flyteidl v0.22.4-0.20220301015125-8c9756723d27 h1:qrqTtWay6wRaJYYapPVeKOqMonsw+AnkYtkoYEj1k4Y=
+github.com/flyteorg/flyteidl v0.22.4-0.20220301015125-8c9756723d27/go.mod h1:576W2ViEyjTpT+kEVHAGbrTP3HARNUZ/eCwrNPmdx9U=
+github.com/flyteorg/flyteidl v0.23.0 h1:Pjl9Tq1pJfIK0au5PiqPVpl25xTYosN6BruZl+PgWAk=
+github.com/flyteorg/flyteidl v0.23.0/go.mod h1:576W2ViEyjTpT+kEVHAGbrTP3HARNUZ/eCwrNPmdx9U=
 github.com/flyteorg/flyteplugins v0.10.9 h1:Hj4kBc2pNAJOTUP14+KSd44RzSE+A6fMEnTMQXhE6r8=
 github.com/flyteorg/flyteplugins v0.10.9/go.mod h1:RXgHGGUGC1akEnAd0yi4cLuYP1BF1rVkxhGjzIrm6VU=
 github.com/flyteorg/flytestdlib v0.3.13/go.mod h1:Tz8JCECAbX6VWGwFT6cmEQ+RJpZ/6L9pswu3fzWs220=

diff --git a/pkg/controller/nodes/task/catalog/datacatalog/transformer.go b/pkg/controller/nodes/task/catalog/datacatalog/transformer.go
@@ -116,13 +116,67 @@ func generateTaskSignatureHash(ctx context.Context, taskInterface core.TypedInte
 	return fmt.Sprintf("%v-%v", inputHashString, outputHashString), nil
 }
 
+// Hashify a literal, in other words, produce a new literal where the corresponding value is removed in case
+// the literal hash is set.
+func hashify(literal *core.Literal) *core.Literal {
+	// Two recursive cases:
+	//   1. A collection of literals or
+	//   2. A map of literals
+
+	if literal.GetCollection() != nil {
+		literals := literal.GetCollection().Literals
+		literalsHash := make([]*core.Literal, 0)
+		for _, lit := range literals {
+			literalsHash = append(literalsHash, hashify(lit))
+		}
+		return &core.Literal{
+			Value: &core.Literal_Collection{
+				Collection: &core.LiteralCollection{
+					Literals: literalsHash,
+				},
+			},
+		}
+	}
+	if literal.GetMap() != nil {
+		literalsMap := make(map[string]*core.Literal)
+		for key, lit := range literal.GetMap().Literals {
+			literalsMap[key] = hashify(lit)
+		}
+		return &core.Literal{
+			Value: &core.Literal_Map{
+				Map: &core.LiteralMap{
+					Literals: literalsMap,
+				},
+			},
+		}
+	}
+
+	// And a base case that consists of a scalar, where the hash might be set
+	if literal.GetHash() != "" {
+		return &core.Literal{
+			Hash: literal.GetHash(),
+		}
+	}
+	return literal
+}
+
 // Generate a tag by hashing the input values
 func GenerateArtifactTagName(ctx context.Context, inputs *core.LiteralMap) (string, error) {
 	if inputs == nil || len(inputs.Literals) == 0 {
 		inputs = &emptyLiteralMap
 	}
 
-	inputsHash, err := pbhash.ComputeHash(ctx, inputs)
+	// Hashify, i.e. generate a copy of the literal map where each literal value is removed
+	// in case the corresponding hash is set.
+	hashifiedLiteralMap := make(map[string]*core.Literal, len(inputs.Literals))
+	for name, literal := range inputs.Literals {
+		hashifiedLiteralMap[name] = hashify(literal)
+	}
+	hashifiedInputs := &core.LiteralMap{
+		Literals: hashifiedLiteralMap,
+	}
+
+	inputsHash, err := pbhash.ComputeHash(ctx, hashifiedInputs)
 	if err != nil {
 		return "", err
 	}