Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(api): resolve all api voilation exceptions in katib api #2482

Merged
merged 1 commit into from
Jan 21, 2025
Merged

fix(api): resolve all api voilation exceptions in katib api #2482

merged 1 commit into from
Jan 21, 2025

Conversation

truc0
Copy link
Contributor

@truc0 truc0 commented Jan 12, 2025

What this PR does / why we need it:

  • resolve all api violation exceptions in katib api in hack/violation_exception_v1beta1

    katib/hack/violation_exception_v1beta1.list

    Lines 1 to 30 in eb8af4d

    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1,AlgorithmSpec,AlgorithmSettings
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1,EarlyStoppingSpec,AlgorithmSettings
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1,FilterSpec,MetricsFormat
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1,ObjectiveSpec,AdditionalMetricNames
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1,ObjectiveSpec,MetricStrategies
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1,Observation,Metrics
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,ExperimentSpec,Parameters
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,ExperimentStatus,Conditions
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,ExperimentStatus,EarlyStoppedTrialList
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,ExperimentStatus,FailedTrialList
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,ExperimentStatus,KilledTrialList
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,ExperimentStatus,MetricsUnavailableTrialList
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,ExperimentStatus,PendingTrialList
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,ExperimentStatus,RunningTrialList
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,ExperimentStatus,SucceededTrialList
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,FeasibleSpace,List
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,GraphConfig,InputSizes
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,GraphConfig,OutputSizes
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,NasConfig,Operations
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,Operation,Parameters
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,OptimalTrial,ParameterAssignments
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1,TrialTemplate,TrialParameters
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/suggestions/v1beta1,SuggestionStatus,AlgorithmSettings
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/suggestions/v1beta1,SuggestionStatus,Conditions
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/suggestions/v1beta1,SuggestionStatus,Suggestions
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/suggestions/v1beta1,TrialAssignment,EarlyStoppingRules
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/suggestions/v1beta1,TrialAssignment,ParameterAssignments
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/trials/v1beta1,TrialSpec,EarlyStoppingRules
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/trials/v1beta1,TrialSpec,ParameterAssignments
    API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/trials/v1beta1,TrialStatus,Conditions

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2479

Checklist:

  • Docs included if any changes are user facing

@truc0 truc0 force-pushed the 2479-resolve-api-violation-exceptions-in-katib-api branch from 896141d to 52d52aa Compare January 12, 2025 14:04
Electronic-Waste
Electronic-Waste reviewed Jan 14, 2025
View reviewed changes
Copy link
Member

@Electronic-Waste Electronic-Waste left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Basically LGTM! Thanks for your great contribution @truc0. Welcome to Kubeflow Community🎉

/assign @kubeflow/wg-automl-leads @helenxie-bit @tariq-hasan

pkg/apis/controller/common/v1beta1/common_types.go Outdated
@@ -27,6 +27,9 @@ type AlgorithmSpec struct {
AlgorithmName string `json:"algorithmName,omitempty"`

// Key-value pairs representing settings for suggestion algorithms.
// +listType=map
// +listMapKey=Name
// +optional
Copy link
Member

Choose a reason for hiding this comment

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

I think, we may not need the +optional tag, and the same as other +optional tag you added:)

Copy link
Contributor Author

@truc0 truc0 Jan 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for pointing out.

I have removed all +optional mark in the updated version (8802b0f)~

PTAL if you have time.

@helenxie-bit
Copy link
Contributor

Thank you for your contribution and welcome to the Kubeflow community! The changes look good and I don’t have additional comments. LGTM! 😄

@Electronic-Waste
Copy link
Member

/rerun-all

@truc0 truc0 force-pushed the 2479-resolve-api-violation-exceptions-in-katib-api branch 2 times, most recently from d92c338 to 8802b0f Compare January 14, 2025 15:31
Electronic-Waste
Electronic-Waste approved these changes Jan 14, 2025
View reviewed changes
Copy link
Member

@Electronic-Waste Electronic-Waste left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for your great contributions!

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Jan 14, 2025
@Electronic-Waste
Copy link
Member

/rerun-all

1 similar comment
@Electronic-Waste
Copy link
Member

/rerun-all

@Electronic-Waste
Copy link
Member

@truc0 The Publish Katib Core Images CI tests sometimes failed due to cache exporting errors.

This PR #2487 raised by @Doris-xm has fixed that error. The CI tests may pass more smoothly if you could rebase your PR:)

@truc0 truc0 force-pushed the 2479-resolve-api-violation-exceptions-in-katib-api branch from 8802b0f to a062fd1 Compare January 15, 2025 06:48
@google-oss-prow google-oss-prow bot removed the lgtm label Jan 15, 2025
@truc0
Copy link
Contributor Author

truc0 commented Jan 15, 2025

Thanks for your advice. The latest version (a062fd1) has rebased.

@Electronic-Waste
Copy link
Member

/ok-to-test

@truc0
Copy link
Contributor Author

truc0 commented Jan 15, 2025

/rerun-all

2 similar comments
@truc0
Copy link
Contributor Author

truc0 commented Jan 15, 2025

/rerun-all

@truc0
Copy link
Contributor Author

truc0 commented Jan 15, 2025

/rerun-all

andreyvelich
andreyvelich reviewed Jan 15, 2025
View reviewed changes
Copy link
Member

@andreyvelich andreyvelich left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution @truc0!

pkg/apis/controller/common/v1beta1/common_types.go Outdated
@@ -27,6 +27,8 @@ type AlgorithmSpec struct {
AlgorithmName string `json:"algorithmName,omitempty"`

// Key-value pairs representing settings for suggestion algorithms.
// +listType=map
// +listMapKey=Name
Copy link
Member

Choose a reason for hiding this comment

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

Should this Name be lowercase, like here: https://github.com/kubeflow/training-operator/blob/master/pkg/apis/kubeflow.org/v2alpha1/trainjob_types.go#L210 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing out! The latest version (34a9991) has updated all listMapKey to lowercase (the same as the key specified in json format).

pkg/apis/controller/experiments/v1beta1/experiment_types.go Outdated
@@ -78,30 +80,39 @@ type ExperimentStatus struct {
LastReconcileTime *metav1.Time `json:"lastReconcileTime,omitempty"`

// List of observed runtime conditions for this Experiment.
// +listType=map
// +listMapKey=Type
Copy link
Member

Choose a reason for hiding this comment

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

Same question for Type.

@truc0 truc0 force-pushed the 2479-resolve-api-violation-exceptions-in-katib-api branch from a062fd1 to 34a9991 Compare January 16, 2025 09:39
@Electronic-Waste
Copy link
Member

/rerun-all

3 similar comments
@truc0
Copy link
Contributor Author

truc0 commented Jan 16, 2025

/rerun-all

@truc0
Copy link
Contributor Author

truc0 commented Jan 16, 2025

/rerun-all

@truc0
Copy link
Contributor Author

truc0 commented Jan 16, 2025

/rerun-all

@truc0
Copy link
Contributor Author

truc0 commented Jan 16, 2025

/rerun-all

1 similar comment
@truc0
Copy link
Contributor Author

truc0 commented Jan 16, 2025

/rerun-all

@truc0 truc0 requested a review from andreyvelich January 17, 2025 15:53
@Electronic-Waste Electronic-Waste mentioned this pull request Jan 21, 2025
Open
7 tasks
@andreyvelich andreyvelich added this to the v0.18 milestone Jan 21, 2025
andreyvelich
andreyvelich reviewed Jan 21, 2025
View reviewed changes
Copy link
Member

@andreyvelich andreyvelich left a comment

Choose a reason for hiding this comment

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

Thank you for this great contribution @truc0!
/lgtm
/approve

@google-oss-prow google-oss-prow bot added the lgtm label Jan 21, 2025
@google-oss-prow Google OSS Prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreyvelich, Electronic-Waste

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 224aa9d into kubeflow:master Jan 21, 2025
63 checks passed
@Electronic-Waste
Copy link
Member

Thanks for your great contribution @truc0 ! And also, welcome to the Kubeflow Community! 🎉

Wish it a memorable journey for you to explore Kubeflow:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreyvelich andreyvelich andreyvelich left review comments

@Electronic-Waste Electronic-Waste Electronic-Waste approved these changes

@anencore94 anencore94 Awaiting requested review from anencore94

Assignees

@andreyvelich andreyvelich

@Electronic-Waste Electronic-Waste

Labels
approved lgtm ok-to-test size/L
Projects
None yet
Milestone
v0.18
Development

Successfully merging this pull request may close these issues.

Resolve API Violation Exceptions in Katib API
4 participants
@truc0 @helenxie-bit @Electronic-Waste @andreyvelich