lint

master/internal/api_tasks_intg_test.go

@@ -219,8 +219,14 @@ func TestPostTaskLogsLogPattern(t *testing.T) {
 	activeConfig, err := api.m.db.ActiveExperimentConfig(trial.ExperimentID)
 	require.NoError(t, err)
 	activeConfig.RawLogPolicies = expconf.LogPoliciesConfig{
-		expconf.LogPolicy{RawPattern: "sub", RawActions: expconf.LogActionsV0{expconf.LogActionV0{Type: expconf.LogActionTypeCancelRetries}}},
-		expconf.LogPolicy{RawPattern: `\d{5}$`, RawActions: expconf.LogActionsV0{expconf.LogActionV0{Type: expconf.LogActionTypeExcludeNode}}},
+		expconf.LogPolicy{
+			RawPattern: "sub",
+			RawActions: expconf.LogActionsV0{expconf.LogActionV0{Type: expconf.LogActionTypeCancelRetries}},
+		},
+		expconf.LogPolicy{
+			RawPattern: `\d{5}$`,
+			RawActions: expconf.LogActionsV0{expconf.LogActionV0{Type: expconf.LogActionTypeExcludeNode}},
+		},
 	}
 
 	v, err := json.Marshal(activeConfig)

master/pkg/model/task_container_defaults.go

@@ -319,10 +319,12 @@ func (c TaskContainerDefaultsConfig) Merge(
 		res.Pbs.SetSbatchArgs(tmp)
 	}
 
-	if res.LogPolicies == nil {
-		res.LogPolicies = other.LogPolicies
-	} else {
-		res.LogPolicies = res.LogPolicies.Merge(other.LogPolicies)
+	if other.LogPolicies != nil {
+		if res.LogPolicies == nil {
+			res.LogPolicies = other.LogPolicies
+		} else {
+			res.LogPolicies = res.LogPolicies.Merge(other.LogPolicies)
+		}
 	}
 
 	if other.PreemptionTimeout > 0 {

master/pkg/model/task_container_defaults_test.go

@@ -449,66 +449,18 @@ func TestLogPatternUnmarshal(t *testing.T) {
 		ShmSizeBytes:      4294967296,
 		NetworkMode:       "bridge",
 		PreemptionTimeout: DefaultPreemptionTimeout,
-		LogPolicies: &expconf.LogPoliciesConfig{
-			expconf.LogPolicy{RawPattern: "test", RawActions: []expconf.LogAction{{
-				RawExcludeNode: &expconf.LogActionExcludeNode{},
-			}}},
-			expconf.LogPolicy{RawPattern: "test2", RawActions: []expconf.LogAction{{
-				RawCancelRetries: &expconf.LogActionCancelRetries{},
-			}}},
-		},
-	}
-	require.Equal(t, expected, tcd)
-}
-
-func TestLogPatternPoliciesMerging(t *testing.T) {
-	defaults := &TaskContainerDefaultsConfig{
-		LogPolicies: &expconf.LogPoliciesConfig{
-			expconf.LogPolicy{RawPattern: "a", RawActions: []expconf.LogAction{{
-				RawCancelRetries: &expconf.LogActionCancelRetries{},
-			}}},
-			expconf.LogPolicy{RawPattern: "b", RawActions: []expconf.LogAction{
-				{
-					RawExcludeNode: &expconf.LogActionExcludeNode{},
-				},
-				{
-					RawCancelRetries: &expconf.LogActionCancelRetries{},
-				},
-			}},
-		},
-	}
-
-	conf := expconf.ExperimentConfig{
-		RawLogPolicies: &expconf.LogPoliciesConfig{
-			expconf.LogPolicy{RawPattern: "b", RawActions: []expconf.LogAction{{
-				RawCancelRetries: &expconf.LogActionCancelRetries{},
-			}}},
-			expconf.LogPolicy{RawPattern: "c", RawActions: []expconf.LogAction{{
-				RawExcludeNode: &expconf.LogActionExcludeNode{},
-			}}},
-		},
-	}
-
-	defaults.MergeIntoExpConfig(&conf)
-
-	expected := &expconf.LogPoliciesConfig{
-		expconf.LogPolicy{RawPattern: "a", RawActions: []expconf.LogAction{{
-			RawCancelRetries: &expconf.LogActionCancelRetries{},
-		}}},
-		expconf.LogPolicy{RawPattern: "b", RawActions: []expconf.LogAction{
-			{
-				RawExcludeNode: &expconf.LogActionExcludeNode{},
+		LogPolicies: expconf.LogPoliciesConfig{
+			expconf.LogPolicy{
+				RawPattern: "test",
+				RawActions: expconf.LogActionsV0{expconf.LogActionV0{Type: expconf.LogActionTypeExcludeNode}},
 			},
-			{
-				RawCancelRetries: &expconf.LogActionCancelRetries{},
+			expconf.LogPolicy{
+				RawPattern: "test2",
+				RawActions: expconf.LogActionsV0{expconf.LogActionV0{Type: expconf.LogActionTypeCancelRetries}},
 			},
-		}},
-		expconf.LogPolicy{RawPattern: "c", RawActions: []expconf.LogAction{{
-			RawExcludeNode: &expconf.LogActionExcludeNode{},
-		}}},
+		},
 	}
-
-	require.Equal(t, expected, conf.RawLogPolicies)
+	require.Equal(t, expected, tcd)
 }
 
 func TestPodSpecsDefaultMerging(t *testing.T) {

master/pkg/schemas/expconf/log_pattern_config.go

@@ -16,7 +16,7 @@ import (
 //go:generate ../gen.sh
 type LogPoliciesConfigV0 []LogPolicyV0
 
-// WithDefaults implements the Defaultable psuedointerface.
+// WithDefaults implements the Defaultable pseudo-interface.
 func (b LogPoliciesConfigV0) WithDefaults() LogPoliciesConfigV0 {
 	cudaOomPattern := CUDAOOMPattern
 	cudaOomSignal := CUDAOOMSignal
@@ -25,14 +25,20 @@ func (b LogPoliciesConfigV0) WithDefaults() LogPoliciesConfigV0 {
 
 	if b == nil {
 		return LogPoliciesConfigV0{
-			LogPolicyV0{RawPattern: cudaOomPattern, RawActions: []LogActionV0{{Type: LogActionTypeSignal, Signal: &cudaOomSignal}}},
-			LogPolicyV0{RawPattern: eccErrorPattern, RawActions: []LogActionV0{{Type: LogActionTypeSignal, Signal: &eccErrorSignal}}},
+			LogPolicyV0{
+				RawPattern: cudaOomPattern,
+				RawActions: []LogActionV0{{Type: LogActionTypeSignal, Signal: &cudaOomSignal}},
+			},
+			LogPolicyV0{
+				RawPattern: eccErrorPattern,
+				RawActions: []LogActionV0{{Type: LogActionTypeSignal, Signal: &eccErrorSignal}},
+			},
 		}
 	}
 	return b
 }
 
-// Merge implements the Mergable psuedo-interface.
+// Merge implements the Mergable pseudo-interface.
 // We appends all LogPolicyV0s to the output slice, but if there are any with the same pattern, we merge
 // their actions and save them as one LogPolicyV0.
 func (b LogPoliciesConfigV0) Merge(
@@ -121,7 +127,7 @@ func (b *LogPoliciesConfigV0) UnmarshalJSON(data []byte) error {
 	for _, lp := range patternToLp {
 		temp = append(temp, lp)
 	}
-	LogPoliciesConfigV0(temp).sort()
+	temp.sort()
 	*b = temp
 
 	return nil
@@ -134,6 +140,7 @@ func (b LogPoliciesConfigV0) sort() {
 	})
 }
 
+// LogActionsV0 is a list of log actions.
 type LogActionsV0 []LogActionV0
 
 // LogPolicyV0 is an action to take if we match against trial logs.
@@ -265,8 +272,10 @@ func (s LogActionsV0) sort() {
 	})
 }
 
+// LogActionType is the type of an action.
 type LogActionType string
 
+// LogActionType refers to the action user can take when a pattern is detected in the log.
 const (
 	LogActionTypeCancelRetries LogActionType = "cancel_retries"
 	LogActionTypeExcludeNode   LogActionType = "exclude_node"
@@ -299,17 +308,20 @@ func (s LogActionV0) MarshalJSON() ([]byte, error) {
 // UnmarshalJSON implements the json.Unmarshaler interface.
 func (s *LogActionV0) UnmarshalJSON(data []byte) error {
 	var action string
-	if err := json.Unmarshal(data, &action); err == nil {
-		switch LogActionType(action) {
-		case LogActionTypeCancelRetries:
-			*s = LogActionV0{Type: LogActionTypeCancelRetries}
-			return nil
-		case LogActionTypeExcludeNode:
-			*s = LogActionV0{Type: LogActionTypeExcludeNode}
-			return nil
-		}
-	} else {
-		return fmt.Errorf("failed to unmarshal log action type CancelRetries and ExcludeNode: %w, data: %q", err, string(data))
+	if err := json.Unmarshal(data, &action); err != nil {
+		return fmt.Errorf(
+			"failed to unmarshal log action type CancelRetries and ExcludeNode: %w, data: %q", err, string(data),
+		)
+	}
+
+	// Handle all the types beside signal
+	switch LogActionType(action) {
+	case LogActionTypeCancelRetries:
+		*s = LogActionV0{Type: LogActionTypeCancelRetries}
+		return nil
+	case LogActionTypeExcludeNode:
+		*s = LogActionV0{Type: LogActionTypeExcludeNode}
+		return nil
 	}
 
 	// Handle Signal

master/pkg/schemas/expconf/schema_test.go

@@ -13,7 +13,6 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/santhosh-tekuri/jsonschema/v2"
-	tassert "github.com/stretchr/testify/assert"
 	"gotest.tools/assert"
 
 	"github.com/determined-ai/determined/master/pkg/schemas"
@@ -22,20 +21,17 @@ import (
 type JSON = interface{}
 
 type SchemaTestCase struct {
-	Name                  string               `json:"name"`
-	SaneAs                *[]string            `json:"sane_as"`
-	CompleteAs            *[]string            `json:"complete_as"`
-	SanityErrors          *map[string][]string `json:"sanity_errors"`
-	CompletenessErrors    *map[string][]string `json:"completeness_errors"`
-	DefaultAs             *string              `json:"default_as"`
-	Defaulted             *JSON                `json:"defaulted"`
-	Case                  JSON                 `json:"case"`
-	MergeAs               *string              `json:"merge_as"`
-	MergeSrc              *JSON                `json:"merge_src"`
-	Merged                *JSON                `json:"merged"`
-	BackwardCompatibility *string              `json:"backward_compatiblity"`
-	Legacy                *JSON                `json:"legacy"`
-	New                   *JSON                `json:"new"`
+	Name               string               `json:"name"`
+	SaneAs             *[]string            `json:"sane_as"`
+	CompleteAs         *[]string            `json:"complete_as"`
+	SanityErrors       *map[string][]string `json:"sanity_errors"`
+	CompletenessErrors *map[string][]string `json:"completeness_errors"`
+	DefaultAs          *string              `json:"default_as"`
+	Defaulted          *JSON                `json:"defaulted"`
+	Case               JSON                 `json:"case"`
+	MergeAs            *string              `json:"merge_as"`
+	MergeSrc           *JSON                `json:"merge_src"`
+	Merged             *JSON                `json:"merged"`
 }
 
 func errorIn(expect string, errors []error) bool {
@@ -318,12 +314,9 @@ func (tc SchemaTestCase) CheckRoundTrip(t *testing.T) {
 	obj := objectForURL(url)
 	err = json.Unmarshal(byts, &obj)
 	assert.NilError(t, err)
-	v, _ := obj.(ExperimentConfigV0)
-	fmt.Printf("obj: %#v\n", v.RawLogPolicies)
 
 	// Round-trip through json.
 	jByts, err := json.Marshal(obj)
-	fmt.Printf("\njson output: %#v\n", string(jByts))
 	assert.NilError(t, err)
 	cpy := objectForURL(url)
 	err = json.Unmarshal(jByts, &cpy)
@@ -343,60 +336,6 @@ func (tc SchemaTestCase) CheckRoundTrip(t *testing.T) {
 	assert.DeepEqual(t, obj, cpy)
 }
 
-func (tc SchemaTestCase) CheckBackwardCompatiblity(t *testing.T) {
-	if tc.BackwardCompatibility == nil && tc.Legacy == nil && tc.New == nil {
-		return
-	}
-
-	if tc.BackwardCompatibility == nil || tc.Legacy == nil || tc.New == nil {
-		assert.NilError(t, errors.New(
-			"if any of backward_compatibility, legacy, or new are set in a test case, "+
-				"they must all be set",
-		))
-	}
-
-	if *tc.BackwardCompatibility != "http://determined.ai/schemas/expconf/v0/log-policies.json" {
-		assert.NilError(t, errors.New("only log-policies.json is supported for now"))
-	}
-
-	legacyBytes, err := json.Marshal(tc.Legacy)
-	assert.NilError(t, err)
-	newBytes, err := json.Marshal(tc.New)
-	assert.NilError(t, err)
-
-	// Get an empty objects to unmarshal into.
-	legacy := LogPoliciesConfig{}
-	new := LogPoliciesConfig{}
-
-	err = json.Unmarshal(legacyBytes, &legacy)
-	assert.NilError(t, err)
-	err = json.Unmarshal(newBytes, &new)
-	assert.NilError(t, err)
-
-	legacyPatterns := make([]string, 0)
-	newPatternToActions := make(map[string][]LogAction)
-	newPatterns := make([]string, 0)
-	for _, lp := range new {
-		newPatternToActions[lp.Pattern()] = lp.Actions()
-		newPatterns = append(newPatterns, lp.Pattern())
-	}
-	for _, lp := range legacy {
-		legacyPatterns = append(legacyPatterns, lp.Pattern())
-	}
-
-	assert := tassert.New(t)
-	assert.ElementsMatch(legacyPatterns, newPatterns, "", "legacy input was't translated to the expected output")
-
-	for _, lp := range legacy {
-		pattern := lp.Pattern()
-
-		legacyActions := lp.Actions()
-		newActions := newPatternToActions[pattern]
-
-		assert.ElementsMatch(legacyActions, newActions, "legacy input was't translated to the expected output")
-	}
-}
-
 func RunCasesFile(t *testing.T, path string, displayPath string) {
 	// Ignore the security error about including files as variables; this is just a test.
 	byts, err := os.ReadFile(path) //nolint: gosec
@@ -419,7 +358,6 @@ func RunCasesFile(t *testing.T, path string, displayPath string) {
 			tc.CheckDefaulted(t)
 			tc.CheckRoundTrip(t)
 			tc.CheckMerged(t)
-			tc.CheckBackwardCompatiblity(t)
 		})
 	}
 }

master/pkg/union/unmarshal.go

@@ -2,7 +2,6 @@ package union
 
 import (
 	"encoding/json"
-	"fmt"
 	"reflect"
 	"strings"
 
@@ -11,7 +10,6 @@ import (
 
 // Unmarshal unmarshals the provided union type from a JSON byte array.
 func Unmarshal(data []byte, v interface{}) error {
-	fmt.Printf("\n\nunion unmarshal is called!\n\n")
 	value := reflect.ValueOf(v)
 	expectedFields := make(map[string]bool)
 	unionTypes, err := parseUnionTypes(value.Type().Elem())