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

[WIP] Add clientset for rayclusters #78

Closed
wants to merge 1 commit into from

Conversation

harryge00
Copy link
Contributor

@harryge00 harryge00 commented Oct 18, 2021

Why are these changes needed?

  1. To solve Support clientset for RayCluster objects  #29
  2. RayCluster is a kubernetes CRD objects we use for generating ray clusters and manage its life cycle. We need using codegen tools to generate clientset, informers automatically.

Related issue number

#29

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

@Jeffwan
Copy link
Collaborator

Jeffwan commented Oct 18, 2021

@harryge00 Thanks for the contribution. I fully understand the background and scope of this PR. Let's add some description in the PR for other reviewer?

Copy link
Collaborator

@Jeffwan Jeffwan left a comment

Choose a reason for hiding this comment

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

  1. We also need a script to generate the clientset. Can you please add it as well?
  2. Just like to confirm we have to restructure api folder. Is that correct? Support clientset for RayCluster objects  #29 (comment)

// +k8s:defaulter-gen=TypeMeta
// +groupName=raycluster

package v1alpha1
Copy link
Collaborator

Choose a reason for hiding this comment

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

empty line after this line

@Jeffwan
Copy link
Collaborator

Jeffwan commented Oct 20, 2021

@harryge00 Can you help address the comments?

@Jeffwan
Copy link
Collaborator

Jeffwan commented Oct 25, 2021

I am thinking maybe we can split PRs into smaller ones?

  • ​Reorganize PR and update reference in controllers and tools.
  • ​Add codegen scripts and api changes (doc.go, spec.go etc)
  • ​Generate clients, listers, informers.

What do you think? @harryge00 @chenk008

@harryge00
Copy link
Contributor Author

@Jeffwan
Sorry for the late reply. I am still struggling with kubebuilder actually... I agree with you, we can split it into smaller ones. For scripts to generating clientset, you mean like https://github.com/knative/pkg/blob/main/hack/update-codegen.sh ?

@harryge00
Copy link
Contributor Author

If this PR blocks u, u can take it and make a small one first. 😭
I can take other ones

@Jeffwan
Copy link
Collaborator

Jeffwan commented Oct 27, 2021

@harryge00 no worries, Could I know what's the problem of kubebuilder? Anything I can help here?

@Jeffwan
Copy link
Collaborator

Jeffwan commented Oct 27, 2021

@harryge00 Can you reach out to me on Ray or Kubernetes Slack channel? I can help you figure this out.

@Jeffwan
Copy link
Collaborator

Jeffwan commented Nov 2, 2021

This can be closed.

Check following PRs instead
#91
#96

@Jeffwan Jeffwan closed this Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants