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.

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

KEP-2170: Change API Group Name to trainer.kubeflow.org #2413

Merged
merged 11 commits into from
Feb 5, 2025

Conversation

Electronic-Waste
Copy link
Member

@Electronic-Waste Electronic-Waste commented Feb 4, 2025

What this PR does / why we need it:

As we discussed in kubeflow/pipelines#11551 (comment), it's necessary to use different api groups in different components to avoid naming conflicts. So I:

  • Change the API Group name to trainer.kubeflow.org
  • Rename /pkg/apis/kubeflow.org to /pkg/apis/trainer
  • Update all related reference, scripts and code

/cc @kubeflow/wg-training-leads @franciscojavierarceo @astefanutti @deepanker13 @saileshd1402 @seanlaii @helenxie-bit @astefanutti @varshaprasad96 @Doris-xm @truc0

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

Checklist:

  • Docs included if any changes are user facing

Copy link

@Electronic-Waste: GitHub didn't allow me to request PR reviews from the following users: saileshd1402, varshaprasad96, astefanutti, seanlaii, truc0.

Note that only kubeflow members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

What this PR does / why we need it:

As we discussed in kubeflow/pipelines#11551 (comment), it's necessary to use different api groups in different components to avoid naming conflicts. So I:

  • Change the API Group name to trainer.kubeflow.org
  • Rename /pkg/apis/kubeflow.org to /pkg/apis/trainer.kubeflow.org
  • Update all related reference, scripts and code

/cc @kubeflow/wg-training-leads @franciscojavierarceo @astefanutti @deepanker13 @saileshd1402 @seanlaii @helenxie-bit @astefanutti @varshaprasad96 @Doris-xm @truc0

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

Checklist:

  • Docs included if any changes are user facing

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@google-oss-prow google-oss-prow bot requested a review from a team February 4, 2025 15:56
@Electronic-Waste Electronic-Waste changed the title Change API Group Name to trainer.kubeflow.org KEP-2170: Change API Group Name to trainer.kubeflow.org Feb 4, 2025
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 updating this @Electronic-Waste!
/assign @tenzen-y @astefanutti @kubeflow/wg-training-leads

Copy link
Member

@andreyvelich andreyvelich Feb 4, 2025

Choose a reason for hiding this comment

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

I notice that JobSet and Kueue doesn't have host name in the files name:

`/pkg/apis/trainer/v1alpha1/...

Copy link
Member

Choose a reason for hiding this comment

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

I'm OK with it in any form if that structure should work fine with a couple of code generation mechanisms.

Copy link
Member Author

Choose a reason for hiding this comment

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

SGTM. It looks more precise.

@Electronic-Waste
Copy link
Member Author

@andreyvelich I've updated the dir name from trainer.kubeflow.org to trainer. The code generation mechanisms also work well. Could you please take a look when you are available?

@Electronic-Waste
Copy link
Member Author

/rerun-all

1 similar comment
@Electronic-Waste
Copy link
Member Author

/rerun-all

@astefanutti
Copy link
Contributor

Thanks @Electronic-Waste!

/lgtm

Copy link

@astefanutti: changing LGTM is restricted to collaborators

In response to this:

Thanks @Electronic-Waste!

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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 @Electronic-Waste!
/lgtm
/assign @tenzen-y

Copy link
Member

@tenzen-y tenzen-y left a comment

Choose a reason for hiding this comment

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

@google-oss-prow google-oss-prow bot removed the lgtm label Feb 5, 2025
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 @Electronic-Waste!
/lgtm
/assign @tenzen-y @astefanutti

@Electronic-Waste
Copy link
Member Author

Overall lgtm
Could you update design as well?
https://github.com/kubeflow/training-operator/tree/master/docs/proposals/2170-kubeflow-training-v2

I've updated it. Please let me know if this PR needs any further fixes.

Copy link
Member

@tenzen-y tenzen-y left a comment

Choose a reason for hiding this comment

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

Awesome
/lgtm
/approve

Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tenzen-y

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 62e958f into kubeflow:master Feb 5, 2025
14 checks passed
@Electronic-Waste Electronic-Waste deleted the refactor/api-group branch February 5, 2025 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants