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

Fix openapi generate rule violation #172

Merged
merged 1 commit into from
Nov 6, 2021
Merged

Fix openapi generate rule violation #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkg/apis/common/v1/openapi_generated.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/apis/common/v1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
// +k8s:deepcopy-gen=true
// JobStatus represents the current observed state of the training Job.
type JobStatus struct {
// +listType=set
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be a map, with listMapKey=type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm. afraid there can be conflicts if it's map with listMapKey=type. For example, there can be multiple items with the same JobCondition like Running. I think there can't be duplicated items so maybe set is suitable?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why would there be multiple conditions for the same type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Condition is a list and it's open to user to set so it may happen though it's not recommended. But you are right I checked code for JobStatus it's impossible this way. I can modify it then.

Copy link
Contributor

Choose a reason for hiding this comment

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

The JobStatus is actually not up to the user, it's entirely controlled by the operators. And if our operators put the same condition more than once, it would be a bug. But generally, lists could act as maps if we decide to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, send another PR #174 to address this.

// Conditions is an array of current observed job conditions.
Conditions []JobCondition `json:"conditions"`

Expand Down