review: change api for new feature

tenzen-y · tenzen-y · commit cb456dd49ea6 · 2022-04-05T23:22:18.000+09:00
diff --git a/cmd/metricscollector/v1beta1/file-metricscollector/main.go b/cmd/metricscollector/v1beta1/file-metricscollector/main.go
@@ -140,7 +140,7 @@ func printMetricsFile(mFile string) {
 	}
 }
 
-func watchMetricsFile(mFile string, stopRules stopRulesFlag, filters []string, fileFormat commonv1beta1.FileSystemFileFormat) {
+func watchMetricsFile(mFile string, stopRules stopRulesFlag, filters []string, fileFormat commonv1beta1.FileFormat) {
 
 	// metricStartStep is the dict where key = metric name, value = start step.
 	// We should apply early stopping rule only if metric is reported at least "start_step" times.
@@ -398,7 +398,7 @@ func main() {
 		filters = strings.Split(*metricFilters, ";")
 	}
 
-	fileFormat := commonv1beta1.FileSystemFileFormat(*metricsFileFormat)
+	fileFormat := commonv1beta1.FileFormat(*metricsFileFormat)
 
 	// If stop rule is set we need to parse metrics during run.
 	if len(stopRules) != 0 {
@@ -425,7 +425,7 @@ func main() {
 	}
 }
 
-func reportMetrics(filters []string, fileFormat commonv1beta1.FileSystemFileFormat) {
+func reportMetrics(filters []string, fileFormat commonv1beta1.FileFormat) {
 
 	conn, err := grpc.Dial(*dbManagerServiceAddr, grpc.WithInsecure())
 	if err != nil {
diff --git a/examples/v1beta1/early-stopping/median-stop-with-json-format.yaml b/examples/v1beta1/early-stopping/median-stop-with-json-format.yaml
@@ -17,7 +17,7 @@ spec:
       fileSystemPath:
         path: "/katib/mnist.json"
         kind: File
-        fileFormat: JSON
+        format: JSON
     collector:
       kind: File
   algorithm:
diff --git a/examples/v1beta1/metrics-collector/file-metrics-collector-with-json-format.yaml b/examples/v1beta1/metrics-collector/file-metrics-collector-with-json-format.yaml
@@ -15,7 +15,7 @@ spec:
       fileSystemPath:
         path: "/katib/mnist.json"
         kind: File
-        fileFormat: JSON
+        format: JSON
     collector:
       kind: File
   algorithm:
diff --git a/pkg/apis/controller/common/v1beta1/common_types.go b/pkg/apis/controller/common/v1beta1/common_types.go
@@ -188,18 +188,18 @@ const (
 	InvalidKind   FileSystemKind = "Invalid"
 )
 
-type FileSystemFileFormat string
+type FileFormat string
 
 const (
-	TextFormat FileSystemFileFormat = "TEXT"
-	JsonFormat FileSystemFileFormat = "JSON"
+	TextFormat FileFormat = "TEXT"
+	JsonFormat FileFormat = "JSON"
 )
 
 // +k8s:deepcopy-gen=true
 type FileSystemPath struct {
-	Path       string               `json:"path,omitempty"`
-	Kind       FileSystemKind       `json:"kind,omitempty"`
-	FileFormat FileSystemFileFormat `json:"fileFormat,omitempty"`
+	Path   string         `json:"path,omitempty"`
+	Kind   FileSystemKind `json:"kind,omitempty"`
+	Format FileFormat     `json:"format,omitempty"`
 }
 
 type CollectorKind string
diff --git a/pkg/apis/controller/experiments/v1beta1/experiment_defaults.go b/pkg/apis/controller/experiments/v1beta1/experiment_defaults.go
@@ -158,8 +158,8 @@ func (e *Experiment) setDefaultMetricsCollector() {
 		if e.Spec.MetricsCollectorSpec.Source.FileSystemPath.Path == "" {
 			e.Spec.MetricsCollectorSpec.Source.FileSystemPath.Path = common.DefaultFilePath
 		}
-		if e.Spec.MetricsCollectorSpec.Source.FileSystemPath.FileFormat == "" {
-			e.Spec.MetricsCollectorSpec.Source.FileSystemPath.FileFormat = common.TextFormat
+		if e.Spec.MetricsCollectorSpec.Source.FileSystemPath.Format == "" {
+			e.Spec.MetricsCollectorSpec.Source.FileSystemPath.Format = common.TextFormat
 		}
 	case common.TfEventCollector:
 		if e.Spec.MetricsCollectorSpec.Source == nil {
diff --git a/pkg/apis/v1beta1/openapi_generated.go b/pkg/apis/v1beta1/openapi_generated.go
diff --git a/pkg/apis/v1beta1/swagger.json b/pkg/apis/v1beta1/swagger.json
@@ -779,7 +779,7 @@
     "v1beta1.FileSystemPath": {
       "type": "object",
       "properties": {
-        "fileFormat": {
+        "format": {
           "type": "string"
         },
         "kind": {
diff --git a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector.go b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector.go
@@ -34,7 +34,7 @@ import (
 	"github.com/kubeflow/katib/pkg/metricscollector/v1beta1/common"
 )
 
-func CollectObservationLog(fileName string, metrics []string, filters []string, fileFormat commonv1beta1.FileSystemFileFormat) (*v1beta1.ObservationLog, error) {
+func CollectObservationLog(fileName string, metrics []string, filters []string, fileFormat commonv1beta1.FileFormat) (*v1beta1.ObservationLog, error) {
 	file, err := os.Open(fileName)
 	if err != nil {
 		return nil, err
diff --git a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go
@@ -44,7 +44,7 @@ func TestCollectObservationLog(t *testing.T) {
 		filePath    string
 		metrics     []string
 		filters     []string
-		fileFormat  commonv1beta1.FileSystemFileFormat
+		fileFormat  commonv1beta1.FileFormat
 		err         bool
 		expected    *v1beta1.ObservationLog
 	}{
diff --git a/pkg/webhook/v1beta1/experiment/validator/validator.go b/pkg/webhook/v1beta1/experiment/validator/validator.go
@@ -428,10 +428,10 @@ func (g *DefaultValidator) validateMetricsCollector(inst *experimentsv1beta1.Exp
 			mcSpec.Source.FileSystemPath.Kind != commonapiv1beta1.FileKind || !filepath.IsAbs(mcSpec.Source.FileSystemPath.Path) {
 			return fmt.Errorf("file path where metrics file exists is required by .spec.metricsCollectorSpec.source.fileSystemPath.path")
 		}
-		// FileSystemFileFormat
-		fileFormat := mcSpec.Source.FileSystemPath.FileFormat
+		// Format
+		fileFormat := mcSpec.Source.FileSystemPath.Format
 		if fileFormat != commonapiv1beta1.TextFormat && fileFormat != commonapiv1beta1.JsonFormat {
-			return fmt.Errorf("format of metrics file is required by .spec.metricsCollectorSpec.source.fileSystemPath.fileFormat")
+			return fmt.Errorf("format of metrics file is required by .spec.metricsCollectorSpec.source.fileSystemPath.format")
 		}
 		if fileFormat == commonapiv1beta1.JsonFormat && mcSpec.Source.Filter != nil {
 			return fmt.Errorf(".spec.metricsCollectorSpec.source.filter must be nil when format of metrics file is %v", commonapiv1beta1.JsonFormat)
@@ -441,8 +441,8 @@ func (g *DefaultValidator) validateMetricsCollector(inst *experimentsv1beta1.Exp
 			mcSpec.Source.FileSystemPath.Kind != commonapiv1beta1.DirectoryKind || !filepath.IsAbs(mcSpec.Source.FileSystemPath.Path) {
 			return fmt.Errorf("directory path where tensorflow event files exist is required by .spec.metricsCollectorSpec.source.fileSystemPath.path")
 		}
-		if mcSpec.Source.FileSystemPath.FileFormat != "" {
-			return fmt.Errorf(".spec.metricsCollectorSpec.source.fileSystemPath.fileFormat must be empty")
+		if mcSpec.Source.FileSystemPath.Format != "" {
+			return fmt.Errorf(".spec.metricsCollectorSpec.source.fileSystemPath.format must be empty")
 		}
 	case commonapiv1beta1.PrometheusMetricCollector:
 		i, err := strconv.Atoi(mcSpec.Source.HttpGet.Port.String())
diff --git a/pkg/webhook/v1beta1/experiment/validator/validator_test.go b/pkg/webhook/v1beta1/experiment/validator/validator_test.go
@@ -798,8 +798,8 @@ func TestValidateMetricsCollector(t *testing.T) {
 					},
 					Source: &commonv1beta1.SourceSpec{
 						FileSystemPath: &commonv1beta1.FileSystemPath{
-							Path:       "not/absolute/path",
-							FileFormat: commonv1beta1.TextFormat,
+							Path:   "not/absolute/path",
+							Format: commonv1beta1.TextFormat,
 						},
 					},
 				}
@@ -837,9 +837,9 @@ func TestValidateMetricsCollector(t *testing.T) {
 					},
 					Source: &commonv1beta1.SourceSpec{
 						FileSystemPath: &commonv1beta1.FileSystemPath{
-							Path:       "/absolute/path",
-							FileFormat: commonv1beta1.JsonFormat,
-							Kind:       commonv1beta1.DirectoryKind,
+							Path:   "/absolute/path",
+							Format: commonv1beta1.JsonFormat,
+							Kind:   commonv1beta1.DirectoryKind,
 						},
 					},
 				}
@@ -942,9 +942,9 @@ func TestValidateMetricsCollector(t *testing.T) {
 							},
 						},
 						FileSystemPath: &commonv1beta1.FileSystemPath{
-							Path:       "/absolute/path",
-							Kind:       commonv1beta1.FileKind,
-							FileFormat: commonv1beta1.TextFormat,
+							Path:   "/absolute/path",
+							Kind:   commonv1beta1.FileKind,
+							Format: commonv1beta1.TextFormat,
 						},
 					},
 				}
@@ -968,9 +968,9 @@ func TestValidateMetricsCollector(t *testing.T) {
 							},
 						},
 						FileSystemPath: &commonv1beta1.FileSystemPath{
-							Path:       "/absolute/path",
-							Kind:       commonv1beta1.FileKind,
-							FileFormat: commonv1beta1.TextFormat,
+							Path:   "/absolute/path",
+							Kind:   commonv1beta1.FileKind,
+							Format: commonv1beta1.TextFormat,
 						},
 					},
 				}
@@ -989,9 +989,9 @@ func TestValidateMetricsCollector(t *testing.T) {
 					},
 					Source: &commonv1beta1.SourceSpec{
 						FileSystemPath: &commonv1beta1.FileSystemPath{
-							Path:       "/absolute/path",
-							Kind:       commonv1beta1.FileKind,
-							FileFormat: "invalid",
+							Path:   "/absolute/path",
+							Kind:   commonv1beta1.FileKind,
+							Format: "invalid",
 						},
 					},
 				}
@@ -1011,9 +1011,9 @@ func TestValidateMetricsCollector(t *testing.T) {
 					Source: &commonv1beta1.SourceSpec{
 						Filter: &commonv1beta1.FilterSpec{},
 						FileSystemPath: &commonv1beta1.FileSystemPath{
-							Path:       "/absolute/path",
-							Kind:       commonv1beta1.FileKind,
-							FileFormat: commonv1beta1.JsonFormat,
+							Path:   "/absolute/path",
+							Kind:   commonv1beta1.FileKind,
+							Format: commonv1beta1.JsonFormat,
 						},
 					},
 				}
@@ -1032,9 +1032,9 @@ func TestValidateMetricsCollector(t *testing.T) {
 					},
 					Source: &commonv1beta1.SourceSpec{
 						FileSystemPath: &commonv1beta1.FileSystemPath{
-							Path:       "/absolute/path",
-							Kind:       commonv1beta1.FileKind,
-							FileFormat: commonv1beta1.JsonFormat,
+							Path:   "/absolute/path",
+							Kind:   commonv1beta1.FileKind,
+							Format: commonv1beta1.JsonFormat,
 						},
 					},
 				}
diff --git a/pkg/webhook/v1beta1/pod/inject_webhook.go b/pkg/webhook/v1beta1/pod/inject_webhook.go
@@ -296,7 +296,7 @@ func (s *SidecarInjector) getMetricsCollectorArgs(trial *trialsv1beta1.Trial, me
 	}
 	if mc.Collector.Kind == common.FileCollector && mc.Source != nil {
 		if mc.Source.FileSystemPath != nil {
-			args = append(args, "-format", string(mc.Source.FileSystemPath.FileFormat))
+			args = append(args, "-format", string(mc.Source.FileSystemPath.Format))
 		}
 	}
 	if mc.Collector.Kind == common.StdOutCollector {
diff --git a/pkg/webhook/v1beta1/pod/inject_webhook_test.go b/pkg/webhook/v1beta1/pod/inject_webhook_test.go
@@ -357,8 +357,8 @@ func TestGetMetricsCollectorArgs(t *testing.T) {
 				},
 				Source: &common.SourceSpec{
 					FileSystemPath: &common.FileSystemPath{
-						Path:       testPath,
-						FileFormat: common.TextFormat,
+						Path:   testPath,
+						Format: common.TextFormat,
 					},
 					Filter: &common.FilterSpec{
 						MetricsFormat: []string{
@@ -389,8 +389,8 @@ func TestGetMetricsCollectorArgs(t *testing.T) {
 				},
 				Source: &common.SourceSpec{
 					FileSystemPath: &common.FileSystemPath{
-						Path:       testPath,
-						FileFormat: common.JsonFormat,
+						Path:   testPath,
+						Format: common.JsonFormat,
 					},
 				},
 			},
diff --git a/sdk/python/v1beta1/docs/V1beta1FileSystemPath.md b/sdk/python/v1beta1/docs/V1beta1FileSystemPath.md
@@ -3,7 +3,7 @@
 ## Properties
 Name | Type | Description | Notes
 ------------ | ------------- | ------------- | -------------
-**file_format** | **str** |  | [optional] 
+**format** | **str** |  | [optional] 
 **kind** | **str** |  | [optional] 
 **path** | **str** |  | [optional] 
 
diff --git a/sdk/python/v1beta1/kubeflow/katib/models/v1beta1_file_system_path.py b/sdk/python/v1beta1/kubeflow/katib/models/v1beta1_file_system_path.py
@@ -33,55 +33,55 @@ class V1beta1FileSystemPath(object):
                             and the value is json key in definition.
     """
     openapi_types = {
-        'file_format': 'str',
+        'format': 'str',
         'kind': 'str',
         'path': 'str'
     }
 
     attribute_map = {
-        'file_format': 'fileFormat',
+        'format': 'format',
         'kind': 'kind',
         'path': 'path'
     }
 
-    def __init__(self, file_format=None, kind=None, path=None, local_vars_configuration=None):  # noqa: E501
+    def __init__(self, format=None, kind=None, path=None, local_vars_configuration=None):  # noqa: E501
         """V1beta1FileSystemPath - a model defined in OpenAPI"""  # noqa: E501
         if local_vars_configuration is None:
             local_vars_configuration = Configuration()
         self.local_vars_configuration = local_vars_configuration
 
-        self._file_format = None
+        self._format = None
         self._kind = None
         self._path = None
         self.discriminator = None
 
-        if file_format is not None:
-            self.file_format = file_format
+        if format is not None:
+            self.format = format
         if kind is not None:
             self.kind = kind
         if path is not None:
             self.path = path
 
     @property
-    def file_format(self):
-        """Gets the file_format of this V1beta1FileSystemPath.  # noqa: E501
+    def format(self):
+        """Gets the format of this V1beta1FileSystemPath.  # noqa: E501
 
 
-        :return: The file_format of this V1beta1FileSystemPath.  # noqa: E501
+        :return: The format of this V1beta1FileSystemPath.  # noqa: E501
         :rtype: str
         """
-        return self._file_format
+        return self._format
 
-    @file_format.setter
-    def file_format(self, file_format):
-        """Sets the file_format of this V1beta1FileSystemPath.
+    @format.setter
+    def format(self, format):
+        """Sets the format of this V1beta1FileSystemPath.
 
 
-        :param file_format: The file_format of this V1beta1FileSystemPath.  # noqa: E501
+        :param format: The format of this V1beta1FileSystemPath.  # noqa: E501
         :type: str
         """
 
-        self._file_format = file_format
+        self._format = format
 
     @property
     def kind(self):

Original file line number	Diff line number	Diff line change
`@@ -140,7 +140,7 @@ func printMetricsFile(mFile string) {`
`140`	`140`	`}`
`141`	`141`	`}`
`142`	`142`
`143`		`-func watchMetricsFile(mFile string, stopRules stopRulesFlag, filters []string, fileFormat commonv1beta1.FileSystemFileFormat) {`
	`143`	`+func watchMetricsFile(mFile string, stopRules stopRulesFlag, filters []string, fileFormat commonv1beta1.FileFormat) {`
`144`	`144`
`145`	`145`	`// metricStartStep is the dict where key = metric name, value = start step.`
`146`	`146`	`// We should apply early stopping rule only if metric is reported at least "start_step" times.`
`@@ -398,7 +398,7 @@ func main() {`
`398`	`398`	`filters = strings.Split(*metricFilters, ";")`
`399`	`399`	`}`
`400`	`400`
`401`		`- fileFormat := commonv1beta1.FileSystemFileFormat(*metricsFileFormat)`
	`401`	`+ fileFormat := commonv1beta1.FileFormat(*metricsFileFormat)`
`402`	`402`
`403`	`403`	`// If stop rule is set we need to parse metrics during run.`
`404`	`404`	`if len(stopRules) != 0 {`
`@@ -425,7 +425,7 @@ func main() {`
`425`	`425`	`}`
`426`	`426`	`}`
`427`	`427`
`428`		`-func reportMetrics(filters []string, fileFormat commonv1beta1.FileSystemFileFormat) {`
	`428`	`+func reportMetrics(filters []string, fileFormat commonv1beta1.FileFormat) {`
`429`	`429`
`430`	`430`	`conn, err := grpc.Dial(*dbManagerServiceAddr, grpc.WithInsecure())`
`431`	`431`	`if err != nil {`
Original file line number	Diff line number	Diff line change
`@@ -158,8 +158,8 @@ func (e *Experiment) setDefaultMetricsCollector() {`
`158`	`158`	`if e.Spec.MetricsCollectorSpec.Source.FileSystemPath.Path == "" {`
`159`	`159`	`e.Spec.MetricsCollectorSpec.Source.FileSystemPath.Path = common.DefaultFilePath`
`160`	`160`	`}`
`161`		`- if e.Spec.MetricsCollectorSpec.Source.FileSystemPath.FileFormat == "" {`
`162`		`- e.Spec.MetricsCollectorSpec.Source.FileSystemPath.FileFormat = common.TextFormat`
	`161`	`+ if e.Spec.MetricsCollectorSpec.Source.FileSystemPath.Format == "" {`
	`162`	`+ e.Spec.MetricsCollectorSpec.Source.FileSystemPath.Format = common.TextFormat`
`163`	`163`	`}`
`164`	`164`	`case common.TfEventCollector:`
`165`	`165`	`if e.Spec.MetricsCollectorSpec.Source == nil {`
Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,7 @@ import (`
`34`	`34`	`"github.com/kubeflow/katib/pkg/metricscollector/v1beta1/common"`
`35`	`35`	`)`
`36`	`36`
`37`		`-func CollectObservationLog(fileName string, metrics []string, filters []string, fileFormat commonv1beta1.FileSystemFileFormat) (*v1beta1.ObservationLog, error) {`
	`37`	`+func CollectObservationLog(fileName string, metrics []string, filters []string, fileFormat commonv1beta1.FileFormat) (*v1beta1.ObservationLog, error) {`
`38`	`38`	`file, err := os.Open(fileName)`
`39`	`39`	`if err != nil {`
`40`	`40`	`return nil, err`
Original file line number	Diff line number	Diff line change
`@@ -428,10 +428,10 @@ func (g DefaultValidator) validateMetricsCollector(inst experimentsv1beta1.Exp`
`428`	`428`	`mcSpec.Source.FileSystemPath.Kind != commonapiv1beta1.FileKind \|\| !filepath.IsAbs(mcSpec.Source.FileSystemPath.Path) {`
`429`	`429`	`return fmt.Errorf("file path where metrics file exists is required by .spec.metricsCollectorSpec.source.fileSystemPath.path")`
`430`	`430`	`}`
`431`		`- // FileSystemFileFormat`
`432`		`- fileFormat := mcSpec.Source.FileSystemPath.FileFormat`
	`431`	`+ // Format`
	`432`	`+ fileFormat := mcSpec.Source.FileSystemPath.Format`
`433`	`433`	`if fileFormat != commonapiv1beta1.TextFormat && fileFormat != commonapiv1beta1.JsonFormat {`
`434`		`- return fmt.Errorf("format of metrics file is required by .spec.metricsCollectorSpec.source.fileSystemPath.fileFormat")`
	`434`	`+ return fmt.Errorf("format of metrics file is required by .spec.metricsCollectorSpec.source.fileSystemPath.format")`
`435`	`435`	`}`
`436`	`436`	`if fileFormat == commonapiv1beta1.JsonFormat && mcSpec.Source.Filter != nil {`
`437`	`437`	`return fmt.Errorf(".spec.metricsCollectorSpec.source.filter must be nil when format of metrics file is %v", commonapiv1beta1.JsonFormat)`
`@@ -441,8 +441,8 @@ func (g DefaultValidator) validateMetricsCollector(inst experimentsv1beta1.Exp`
`441`	`441`	`mcSpec.Source.FileSystemPath.Kind != commonapiv1beta1.DirectoryKind \|\| !filepath.IsAbs(mcSpec.Source.FileSystemPath.Path) {`
`442`	`442`	`return fmt.Errorf("directory path where tensorflow event files exist is required by .spec.metricsCollectorSpec.source.fileSystemPath.path")`
`443`	`443`	`}`
`444`		`- if mcSpec.Source.FileSystemPath.FileFormat != "" {`
`445`		`- return fmt.Errorf(".spec.metricsCollectorSpec.source.fileSystemPath.fileFormat must be empty")`
	`444`	`+ if mcSpec.Source.FileSystemPath.Format != "" {`
	`445`	`+ return fmt.Errorf(".spec.metricsCollectorSpec.source.fileSystemPath.format must be empty")`
`446`	`446`	`}`
`447`	`447`	`case commonapiv1beta1.PrometheusMetricCollector:`
`448`	`448`	`i, err := strconv.Atoi(mcSpec.Source.HttpGet.Port.String())`