Skip to content

Commit

Permalink
Revert "Improve execution name readability (#5637)"
Browse files Browse the repository at this point in the history
This reverts commit 2ed2408.
  • Loading branch information
pingsutw committed Sep 11, 2024
1 parent 8dda600 commit b0d37db
Show file tree
Hide file tree
Showing 13 changed files with 60 additions and 112 deletions.
1 change: 0 additions & 1 deletion flyteadmin/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ require (
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.9.0
github.com/wI2L/jsondiff v0.5.0
github.com/wolfeidau/humanhash v1.1.0
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.47.0
go.opentelemetry.io/otel v1.24.0
golang.org/x/oauth2 v0.16.0
Expand Down
2 changes: 0 additions & 2 deletions flyteadmin/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1297,8 +1297,6 @@ github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6Kllzaw
github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc=
github.com/wI2L/jsondiff v0.5.0 h1:RRMTi/mH+R2aXcPe1VYyvGINJqQfC3R+KSEakuU1Ikw=
github.com/wI2L/jsondiff v0.5.0/go.mod h1:qqG6hnK0Lsrz2BpIVCxWiK9ItsBCpIZQiv0izJjOZ9s=
github.com/wolfeidau/humanhash v1.1.0 h1:06KgtyyABJGBbrfMONrW7S+b5TTYVyrNB/jss5n7F3E=
github.com/wolfeidau/humanhash v1.1.0/go.mod h1:jkpynR1bfyfkmKEQudIC0osWKynFAoayRjzH9OJdVIg=
github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c/go.mod h1:lB8K/P019DLNhemzwFU4jHLhdvlE6uDZjXFejJXr49I=
github.com/xdg/stringprep v0.0.0-20180714160509-73f8eece6fdc/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0HrGL1Y=
github.com/xdg/stringprep v1.0.0/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0HrGL1Y=
Expand Down
5 changes: 3 additions & 2 deletions flyteadmin/pkg/async/schedule/aws/workflow_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (

"github.com/flyteorg/flyte/flyteadmin/pkg/async"
scheduleInterfaces "github.com/flyteorg/flyte/flyteadmin/pkg/async/schedule/interfaces"
"github.com/flyteorg/flyte/flyteadmin/pkg/common/naming"
"github.com/flyteorg/flyte/flyteadmin/pkg/common"
"github.com/flyteorg/flyte/flyteadmin/pkg/errors"
"github.com/flyteorg/flyte/flyteadmin/pkg/manager/interfaces"
runtimeInterfaces "github.com/flyteorg/flyte/flyteadmin/pkg/runtime/interfaces"
Expand Down Expand Up @@ -129,7 +129,7 @@ func generateExecutionName(launchPlan *admin.LaunchPlan, kickoffTime time.Time)
Name: launchPlan.Id.Name,
})
randomSeed := kickoffTime.UnixNano() + int64(hashedIdentifier)
return naming.GetExecutionName(randomSeed)
return common.GetExecutionName(randomSeed)
}

func (e *workflowExecutor) formulateExecutionCreateRequest(
Expand Down Expand Up @@ -207,6 +207,7 @@ func (e *workflowExecutor) run() error {
continue
}
executionRequest := e.formulateExecutionCreateRequest(launchPlan, scheduledWorkflowExecutionRequest.KickoffTime)

ctx = contextutils.WithWorkflowID(ctx, fmt.Sprintf(workflowIdentifierFmt, executionRequest.Project,
executionRequest.Domain, executionRequest.Name))
err = e.resolveKickoffTimeArg(scheduledWorkflowExecutionRequest, launchPlan, executionRequest)
Expand Down
13 changes: 13 additions & 0 deletions flyteadmin/pkg/common/executions.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,22 @@
package common

import (
"fmt"

"k8s.io/apimachinery/pkg/util/rand"

"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
)

const ExecutionIDLength = 20
const ExecutionStringFormat = "a%s"

/* #nosec */
func GetExecutionName(seed int64) string {
rand.Seed(seed)
return fmt.Sprintf(ExecutionStringFormat, rand.String(ExecutionIDLength-1))
}

var terminalExecutionPhases = map[core.WorkflowExecution_Phase]bool{
core.WorkflowExecution_SUCCEEDED: true,
core.WorkflowExecution_FAILED: true,
Expand Down
23 changes: 23 additions & 0 deletions flyteadmin/pkg/common/executions_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package common

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
)

const AllowedExecutionIDStartCharStr = "abcdefghijklmnopqrstuvwxyz"
const AllowedExecutionIDStr = "abcdefghijklmnopqrstuvwxyz1234567890"

var AllowedExecutionIDStartChars = []rune(AllowedExecutionIDStartCharStr)
var AllowedExecutionIDChars = []rune(AllowedExecutionIDStr)

func TestGetExecutionName(t *testing.T) {
randString := GetExecutionName(time.Now().UnixNano())
assert.Len(t, randString, ExecutionIDLength)
assert.Contains(t, AllowedExecutionIDStartChars, rune(randString[0]))
for i := 1; i < len(randString); i++ {
assert.Contains(t, AllowedExecutionIDChars, rune(randString[i]))
}
}
30 changes: 0 additions & 30 deletions flyteadmin/pkg/common/naming/execution_name.go

This file was deleted.

64 changes: 0 additions & 64 deletions flyteadmin/pkg/common/naming/execution_name_test.go

This file was deleted.

3 changes: 1 addition & 2 deletions flyteadmin/pkg/manager/impl/util/shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"google.golang.org/grpc/codes"

"github.com/flyteorg/flyte/flyteadmin/pkg/common"
"github.com/flyteorg/flyte/flyteadmin/pkg/common/naming"
"github.com/flyteorg/flyte/flyteadmin/pkg/errors"
"github.com/flyteorg/flyte/flyteadmin/pkg/manager/impl/shared"
"github.com/flyteorg/flyte/flyteadmin/pkg/manager/impl/validation"
Expand All @@ -26,7 +25,7 @@ func GetExecutionName(request *admin.ExecutionCreateRequest) string {
if request.Name != "" {
return request.Name
}
return naming.GetExecutionName(time.Now().UnixNano())
return common.GetExecutionName(time.Now().UnixNano())
}

func GetTask(ctx context.Context, repo repoInterfaces.Repository, identifier *core.Identifier) (
Expand Down
4 changes: 2 additions & 2 deletions flyteadmin/pkg/manager/impl/util/shared_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (
"github.com/stretchr/testify/assert"
"google.golang.org/grpc/codes"

"github.com/flyteorg/flyte/flyteadmin/pkg/common"
commonMocks "github.com/flyteorg/flyte/flyteadmin/pkg/common/mocks"
"github.com/flyteorg/flyte/flyteadmin/pkg/common/naming"
flyteAdminErrors "github.com/flyteorg/flyte/flyteadmin/pkg/errors"
"github.com/flyteorg/flyte/flyteadmin/pkg/manager/impl/testutils"
managerInterfaces "github.com/flyteorg/flyte/flyteadmin/pkg/manager/interfaces"
Expand Down Expand Up @@ -42,7 +42,7 @@ func TestPopulateExecutionID(t *testing.T) {
Domain: "domain",
})
assert.NotEmpty(t, name)
assert.Len(t, name, naming.ExecutionIDLength)
assert.Len(t, name, common.ExecutionIDLength)
}

func TestPopulateExecutionID_ExistingName(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ type PostgresConfig struct {
}

type FeatureGates struct {
EnableArtifacts bool `json:"enableArtifacts" pflag:",Enable artifacts feature."`
EnableFriendlyNames bool `json:"enableFriendlyNames" pflag:",Enable generation of friendly execution names feature."`
EnableArtifacts bool `json:"enableArtifacts" pflag:",Enable artifacts feature."`
}

// ApplicationConfig is the base configuration to start admin
Expand Down
21 changes: 17 additions & 4 deletions flyteadmin/scheduler/executor/executor_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package executor

import (
"context"
"strings"
"time"

"github.com/prometheus/client_golang/prometheus"
Expand All @@ -11,7 +12,7 @@ import (
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/util/retry"

"github.com/flyteorg/flyte/flyteadmin/pkg/common/naming"
"github.com/flyteorg/flyte/flyteadmin/scheduler/identifier"
"github.com/flyteorg/flyte/flyteadmin/scheduler/repositories/models"
"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/admin"
"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
Expand Down Expand Up @@ -52,11 +53,23 @@ func (w *executor) Execute(ctx context.Context, scheduledTime time.Time, s model
}
}

executionName := naming.GetExecutionName(time.Now().UnixNano())
// Making the identifier deterministic using the hash of the identifier and scheduled time
executionIdentifier, err := identifier.GetExecutionIdentifier(ctx, core.Identifier{

Check failure on line 57 in flyteadmin/scheduler/executor/executor_impl.go

View workflow job for this annotation

GitHub Actions / compile

cannot use core.Identifier{…} (value of type core.Identifier) as *core.Identifier value in argument to identifier.GetExecutionIdentifier
Project: s.Project,
Domain: s.Domain,
Name: s.Name,
Version: s.Version,
}, scheduledTime)

if err != nil {
logger.Errorf(ctx, "failed to generate execution identifier for schedule %+v due to %v", s, err)
return err
}

executionRequest := &admin.ExecutionCreateRequest{
Project: s.Project,
Domain: s.Domain,
Name: executionName,
Name: "f" + strings.ReplaceAll(executionIdentifier.String(), "-", "")[:19],
Spec: &admin.ExecutionSpec{
LaunchPlan: &core.Identifier{
ResourceType: core.ResourceType_LAUNCH_PLAN,
Expand Down Expand Up @@ -84,7 +97,7 @@ func (w *executor) Execute(ctx context.Context, scheduledTime time.Time, s model

// Do maximum of 30 retries on failures with constant backoff factor
opts := wait.Backoff{Duration: 3000, Factor: 2.0, Steps: 30}
err := retry.OnError(opts,
err = retry.OnError(opts,
func(err error) bool {
// For idempotent behavior ignore the AlreadyExists error which happens if we try to schedule a launchplan
// for execution at the same time which is already available in admin.
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ require (
github.com/tidwall/pretty v1.2.0 // indirect
github.com/tidwall/sjson v1.2.5 // indirect
github.com/wI2L/jsondiff v0.5.0 // indirect
github.com/wolfeidau/humanhash v1.1.0 // indirect
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.47.0 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.46.1 // indirect
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1333,8 +1333,6 @@ github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6Kllzaw
github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc=
github.com/wI2L/jsondiff v0.5.0 h1:RRMTi/mH+R2aXcPe1VYyvGINJqQfC3R+KSEakuU1Ikw=
github.com/wI2L/jsondiff v0.5.0/go.mod h1:qqG6hnK0Lsrz2BpIVCxWiK9ItsBCpIZQiv0izJjOZ9s=
github.com/wolfeidau/humanhash v1.1.0 h1:06KgtyyABJGBbrfMONrW7S+b5TTYVyrNB/jss5n7F3E=
github.com/wolfeidau/humanhash v1.1.0/go.mod h1:jkpynR1bfyfkmKEQudIC0osWKynFAoayRjzH9OJdVIg=
github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c/go.mod h1:lB8K/P019DLNhemzwFU4jHLhdvlE6uDZjXFejJXr49I=
github.com/xdg/stringprep v0.0.0-20180714160509-73f8eece6fdc/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0HrGL1Y=
github.com/xdg/stringprep v1.0.0/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0HrGL1Y=
Expand Down

0 comments on commit b0d37db

Please sign in to comment.