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

feat: Bump go version to 1.21 #303

Closed
wants to merge 33 commits into from
Closed

feat: Bump go version to 1.21 #303

wants to merge 33 commits into from

Conversation

ishaansehgal99
Copy link
Collaborator

No description provided.

ishaansehgal99 and others added 22 commits March 19, 2024 21:00
…Destination (#304)

Open to feedback. Here is validation checks for the new CRD.

Code Coverage: 87.7% - workspace_validation.go


[coverage.txt](https://github.com/Azure/kaito/files/14661460/coverage.txt)
I'm assuming this should be `.PHONY` not `.PHONE`

Signed-off-by: Dave Fellows <[email protected]>
Correct the mail list name.

Fix issue #314

Signed-off-by: Fei Guo <[email protected]>
- Add report bug template
- Add feature request template
- Add PR template

Signed-off-by: Heba Elayoty <[email protected]>
Signed-off-by: Ishaan Sehgal <[email protected]>
Co-authored-by: Fei Guo <[email protected]>
Signed-off-by: Ishaan Sehgal <[email protected]>
**Reason for Change**:
- Automate the release workflow.
- Use [Reusable
workflows](https://docs.github.com/en/actions/using-workflows/reusing-workflows#reusable-workflows-and-starter-workflows)
instead of workflow_dispatch. This will enhance the usability of e2e
pipeline.
- Remove unnecessary jobs.
- Fix goreleaser to pick the current tag.
- Add option to update k8s version for upcoming pipelines.

**Requirements**
- [ ] added unit tests and e2e tests (if applicable).

**Issue Fixed**:
<!-- If this PR fixes GitHub issue 4321, add "Fixes #4321" to the next
line. -->

**Notes for Reviewers**:

---------

Signed-off-by: Heba Elayoty <[email protected]>
**Reason for Change**:
Update helm chart to use `Release.Namespace` instead of chart name.

**Requirements**

- [ ] added unit tests and e2e tests (if applicable).

**Issue Fixed**:
<!-- If this PR fixes GitHub issue 4321, add "Fixes #4321" to the next
line. -->

**Notes for Reviewers**:

Signed-off-by: Heba Elayoty <[email protected]>
**Reason for Change**:
Remove PR trigger from release workflow. As we follow a branch based
approach.

**Requirements**

- [ ] added unit tests and e2e tests (if applicable).

**Issue Fixed**:
<!-- If this PR fixes GitHub issue 4321, add "Fixes #4321" to the next
line. -->

**Notes for Reviewers**:

Signed-off-by: Heba Elayoty <[email protected]>
**Reason for Change**:
- The job output for registry was missing.
- calling the output should use the job name, not step one.
- 
**Requirements**

- [ ] added unit tests and e2e tests (if applicable).

**Issue Fixed**:
<!-- If this PR fixes GitHub issue 4321, add "Fixes #4321" to the next
line. -->

**Notes for Reviewers**:

---------

Signed-off-by: Heba <[email protected]>
**Reason for Change**:
This helm chart provides a straightforward example for deploying a
sample UI on top of a kaito workspace.
**Reason for Change**:
This change adds plenty more documentation and OpenAPI spec for our
inference. It also enables the use of preferred non-NVIDIA nodes without
crashing.

**Requirements**

- [x] added unit tests and e2e tests (if applicable).

**Issue Fixed**:
Fixes #321 

**Notes for Reviewers**:
**Reason for Change**:
- Update the namespace to use the release namespace.

**Requirements**

- [ ] added unit tests and e2e tests (if applicable).

**Issue Fixed**:
Fixes #336 
**Notes for Reviewers**:

---------

Signed-off-by: Heba <[email protected]>
Co-authored-by: ishaansehgal99 <[email protected]>
**Reason for Change**:
Address some missing needed nil checks in workspace controller
**Reason for Change**:
Provide a default configmaps in the helm chart.

**Notes for Reviewers**:
An env variable release namespace is passed in for future use so that
the controller will be able to find where the configmap is deployed.

---------

Signed-off-by: Ishaan Sehgal <[email protected]>
**Reason for Change**:
Add some future needed util functions, and move tests to seperate
package to prevent cyclical depdencies where API imports utils then
utils imports API. In order to use utils in API need to seperate out
test logic.
ishaansehgal99 and others added 8 commits April 16, 2024 16:59
**Reason for Change**:
Add global client, accessible via webhooks

In this PR I also
Separate setup and execution phase of webhook server, allows us to
handle setup errors and setup context before attempting to run the
webhook server
**Reason for Change**:
Add util function for configmapmount and getting release namespace.
**Reason for Change**:
This change removes hostPath field in DataSource and DataDestination
struct. Instead, a standard K8s VolumeSource API is used to specify the
volumes used by the tuning Job. This will provide the highest
flexibility for users to load/store the data. HostPath field is hard to
use in practice.

This change also renames the Config to ConfigTemplate because the final
configmap used by the tuning job will be generated by the controller
based on the template. The new name is clearer.

**Notes for Reviewers**:
We will revisit how to do validation check for the Volume field because
it is a fairly complicated API.
**Reason for Change**:
This PR adds two simple checks to the tuning configmap - makes sure
based on the method specified (LoRa or QLoRa) the correct params are
included. Also checks to make sure all the sections specified in the
configmap are recognized.
…/logging - Part 7 (#358)

**Reason for Change**:
This PR introduces some of the util functions that are to be used by the
function

func CreatePresetTuning(ctx context.Context, workspaceObj
*kaitov1alpha1.Workspace,
tuningObj *model.PresetParam, kubeClient client.Client) (client.Object,
error)
	

This function will be in charge of handling launching the training job
using the dataset source and uploading results to dataset destination.
It is also incharge of ensuring configmap tuning parameters are in the
right namespace so they are ready to be used by the fine_tuning_api.py
file.
**Reason for Change**:
Adding a CloudSKUHandler interface that will be used to modularize gpu
skus across multiple cloud platforms

**Requirements**

- [ ] added unit tests and e2e tests (if applicable).

**Issue Fixed**:
<!-- If this PR fixes GitHub issue 4321, add "Fixes #4321" to the next
line. -->

**Notes for Reviewers**:
The cloud specific implementations and fixes/updates to current code
will be part of future PRs.
**Reason for Change**:
Couple UTs added

**Requirements**

- [ x] added unit tests and e2e tests (if applicable).
@helayoty helayoty changed the title feat: Bump go version feat: Bump go version to 1.21 Apr 25, 2024
@ishaansehgal99
Copy link
Collaborator Author

different PR will bump to go 1.22

@helayoty helayoty deleted the Ishaan/bump-go branch May 8, 2024 00:16
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.

5 participants