search

kubernetes-sigs / jobset

JobSet: a k8s native API for distributed ML training and HPC workloads
https://jobset.sigs.k8s.io/
Apache License 2.0
138 stars 46 forks source link

Generate client-go/sdk only if api changed. #222

Closed kannon92 closed 1 year ago

kannon92 commented 1 year ago

Running unit tests/integration tests will now require regeneration of client-go/sdk every time. I think we should try to make this a bit smarter as it does make unit or integration tests take a lot longer.

I see two options for development:

1) Only build client-go/sdk if the api was changed. 2) Move building of client-go/sdk into a separate target and call it separately from make or make test or make test-integration.

Currently it is built alongside make so it will run on every build.

danielvegamyhre commented 1 year ago

+1 on this

kannon92 commented 1 year ago

@danielvegamyhre which preference would you have for this?

I can see benefits of logic to detect changes being useful but I also wonder if making the client generation explicit and just detecting that api is different would be less brittle and having a check to see if api mismatches and just telling the user they need to run generate step..

danielvegamyhre commented 1 year ago

Yeah I think option 2 would be the most straight forward. We could make the change detection part of the verify step in the CI.

kannon92 commented 1 year ago

/good-first-issue /help

k8s-ci-robot commented 1 year ago

@kannon92: This request has been marked as suitable for new contributors.

Guidelines

Please ensure that the issue body includes answers to the following questions:

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-good-first-issue command.

In response to [this](https://github.com/kubernetes-sigs/jobset/issues/222): >/good-first-issue >/help Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
a-hilaly commented 1 year ago

I'd like to give to this one a shot :) /assign

a-hilaly commented 1 year ago

/close

k8s-ci-robot commented 1 year ago

@a-hilaly: You can't close an active issue/PR unless you authored it or you are a collaborator.

In response to [this](https://github.com/kubernetes-sigs/jobset/issues/222#issuecomment-1644431365): >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.