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

merge kubeflow/common.v1 to mpi-operator #571

Merged
merged 7 commits into from
Jul 8, 2023
Merged

merge kubeflow/common.v1 to mpi-operator #571

merged 7 commits into from
Jul 8, 2023

Conversation

lowang-bh
Copy link
Member

@lowang-bh lowang-bh commented Jun 22, 2023 •
edited
Loading

Fix #564

TODO:

  • hack/python-sdk/gen-sdk.sh did not finished due to lack of java runtime.
hack/python-sdk/gen-sdk.sh
Generating V2 OpenAPI specification ...
Generating V2 swagger file ...
Downloading the swagger-codegen JAR package ...
Generating V2 Python SDK for Kubeflow MPI-Operator ...
The operation couldn’t be completed. Unable to locate a Java Runtime.
Please visit http://www.java.com for information on installing Java.

@lowang-bh
Copy link
Member Author

@terrytangyuan @tenzen-y would you please take a look at this pr? thanks.

@tenzen-y
Copy link
Member

My review queue is full. So I will review this PR this weekend or next week.

@lowang-bh
merge kubeflow/common.v1 to mpi-operator
c60a947 
Signed-off-by: lowang_bh <[email protected]>

java gen Python SDK

Signed-off-by: lowang_bh <[email protected]>
tenzen-y
tenzen-y reviewed Jul 3, 2023
View reviewed changes
pkg/apis/kubeflow/v2beta1/common_types.go Outdated Show resolved Hide resolved
pkg/apis/kubeflow/v2beta1/common_types.go Outdated Show resolved Hide resolved
pkg/apis/kubeflow/v2beta1/constants.go Show resolved Hide resolved
tenzen-y
tenzen-y reviewed Jul 5, 2023
View reviewed changes
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.

@lowang-bh Overall LGTM

pkg/apis/kubeflow/v2beta1/types.go Outdated Show resolved Hide resolved
@tenzen-y
Copy link
Member

tenzen-y commented Jul 5, 2023

@lowang-bh Can you fix CI errors?

?   	github.com/kubeflow/mpi-operator/pkg/version	[no test files]
# github.com/kubeflow/mpi-operator/test/integration [github.com/kubeflow/mpi-operator/test/integration.test]
Error: test/integration/mpi_job_controller_test.go:734:50: undefined: common
FAIL	github.com/kubeflow/mpi-operator/test/integration [build failed]
FAIL
make: *** [Makefile:69: test] Error 2
Error: Process completed with exit code 2.

https://github.com/kubeflow/mpi-operator/actions/runs/5462212552/jobs/9941258951?pr=571#step:8:277

...
go: downloading github.com/prometheus/procfs v0.7.3
go: downloading github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369
# github.com/kubeflow/mpi-operator/test/e2e [github.com/kubeflow/mpi-operator/test/e2e.test]
Error: test/e2e/mpi_job_test.go:460:7: undefined: common
FAIL	github.com/kubeflow/mpi-operator/test/e2e [build failed]

https://github.com/kubeflow/mpi-operator/actions/runs/5462212552/jobs/9941259352?pr=571#step:4:3326

tenzen-y
tenzen-y reviewed Jul 5, 2023
View reviewed changes
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.

LGTM
/assign @alculquicondor

alculquicondor
alculquicondor reviewed Jul 5, 2023
View reviewed changes
pkg/apis/kubeflow/v2beta1/constants.go Outdated Show resolved Hide resolved
pkg/apis/kubeflow/v2beta1/types.go Outdated Show resolved Hide resolved
@tenzen-y
Copy link
Member

tenzen-y commented Jul 7, 2023

cc: @alculquicondor

alculquicondor
alculquicondor reviewed Jul 7, 2023
View reviewed changes
Copy link
Collaborator

@alculquicondor alculquicondor left a comment

Choose a reason for hiding this comment

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

/approve

terrytangyuan
terrytangyuan approved these changes Jul 7, 2023
View reviewed changes
Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

Thanks!

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Jul 7, 2023
@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, terrytangyuan

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:
  • OWNERS [alculquicondor,terrytangyuan]

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

@tenzen-y
Copy link
Member

tenzen-y commented Jul 8, 2023

@terrytangyuan @alculquicondor Could you restart build / E2E (v1.27.1) (pull_request) ?

@terrytangyuan
Copy link
Member

Done

@google-oss-prow google-oss-prow bot merged commit e1590ce into kubeflow:master Jul 8, 2023
9 checks passed
@lowang-bh lowang-bh mentioned this pull request Aug 31, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alculquicondor alculquicondor alculquicondor left review comments

@tenzen-y tenzen-y tenzen-y left review comments

@terrytangyuan terrytangyuan terrytangyuan approved these changes

@zw0610 zw0610 Awaiting requested review from zw0610

Assignees

@alculquicondor alculquicondor

@terrytangyuan terrytangyuan

Labels
approved lgtm size/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Copy APIs from common repo into here
4 participants
@lowang-bh @tenzen-y @terrytangyuan @alculquicondor