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

Update scripts to generate sdk for all frameworks #1389

Merged
merged 3 commits into from
Sep 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,6 @@ examples/.ipynb_checkpoints/

**/.ipynb_checkpoints

# swagger-codegen tools and auto generated files but useless
hack/python-sdk/swagger-codegen-cli.jar
# openapi-codegen tools and auto generated files but useless
hack/python-sdk/openapi-generator-cli.jar

44 changes: 27 additions & 17 deletions hack/python-sdk/gen-sdk.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,33 +18,43 @@ set -o errexit
set -o nounset
set -o pipefail

SWAGGER_JAR_URL="http://search.maven.org/maven2/io/swagger/swagger-codegen-cli/2.4.6/swagger-codegen-cli-2.4.6.jar"
SWAGGER_CODEGEN_JAR="hack/python-sdk/swagger-codegen-cli.jar"
SWAGGER_JAR_URL="https://repo1.maven.org/maven2/org/openapitools/openapi-generator-cli/4.3.1/openapi-generator-cli-4.3.1.jar"
SWAGGER_CODEGEN_JAR="hack/python-sdk/openapi-generator-cli.jar"
SWAGGER_CODEGEN_CONF="hack/python-sdk/swagger_config.json"
SWAGGER_CODEGEN_FILE="pkg/apis/tensorflow/v1/swagger.json"
SDK_OUTPUT_PATH="sdk/python"
SDK_OUTPUT_PATH="/tmp/sdk/python"
FRAMEWORKS=(tensorflow pytorch mxnet xgboost)
VERSION=1.3.0

if [ -z "${GOPATH:-}" ]; then
export GOPATH=$(go env GOPATH)
fi

# TBD (@jinchihe) This is not consistent with current generation in hack/update-codegen.sh.
# Need to confirm current one is useful. To aviod breaking current one, backup and rollback.
mv pkg/apis/tensorflow/v1/openapi_generated.go pkg/apis/tensorflow/v1/openapi_generated.go.backup

echo "Generating OpenAPI specification ..."
go run vendor/k8s.io/code-generator/cmd/openapi-gen/main.go --input-dirs github.com/kubeflow/tf-operator/pkg/apis/tensorflow/v1,github.com/kubeflow/common/job_controller/api/v1 --output-package github.com/kubeflow/tf-operator/pkg/apis/tensorflow/v1 --go-header-file hack/boilerplate/boilerplate.go.txt

echo "Generating swagger file ..."
go run hack/python-sdk/main.go 0.1 > ${SWAGGER_CODEGEN_FILE}
echo "./hack/update-codegen.sh already help us generate openapi specs ..."

echo "Downloading the swagger-codegen JAR package ..."
wget -O ${SWAGGER_CODEGEN_JAR} ${SWAGGER_JAR_URL}

echo "Generating Python SDK for Kubeflow TF-Operator ..."
java -jar ${SWAGGER_CODEGEN_JAR} generate -i ${SWAGGER_CODEGEN_FILE} -l python -o ${SDK_OUTPUT_PATH} -c ${SWAGGER_CODEGEN_CONF}

# Rollback the current openapi_generated.go
mv pkg/apis/tensorflow/v1/openapi_generated.go.backup pkg/apis/tensorflow/v1/openapi_generated.go
for FRAMEWORK in ${FRAMEWORKS[@]}; do
SWAGGER_CODEGEN_FILE="pkg/apis/${FRAMEWORK}/v1/swagger.json"
echo "Generating swagger file for ${FRAMEWORK} ..."
go run hack/python-sdk/main.go ${FRAMEWORK} ${VERSION} > ${SWAGGER_CODEGEN_FILE}
done

echo "Merging swagger files from different frameworks into one"
download_url=$(curl -s https://api.github.com/repos/go-swagger/go-swagger/releases/latest | \
jq -r '.assets[] | select(.name | contains("'"$(uname | tr '[:upper:]' '[:lower:]')"'_amd64")) | .browser_download_url')
curl -o /tmp/swagger -L'#' "$download_url"
chmod +x /tmp/swagger

# it will report warning like 'v1.SchedulingPolicy' already exists in primary or higher priority mixin, skipping
# error code is not 0 but t's acceptable.
/tmp/swagger mixin pkg/apis/tensorflow/v1/swagger.json pkg/apis/pytorch/v1/swagger.json pkg/apis/mxnet/v1/swagger.json pkg/apis/xgboost/v1/swagger.json \
--output hack/python-sdk/swagger.json --quiet || true

echo "Generating Python SDK for ${FRAMEWORK} ..."
java -jar ${SWAGGER_CODEGEN_JAR} generate -i hack/python-sdk/swagger.json -g python -o ${SDK_OUTPUT_PATH} -c ${SWAGGER_CODEGEN_CONF}

echo "Kubeflow TF-Operator Python SDK is generated successfully to folder ${SDK_OUTPUT_PATH}/."
echo "Kubeflow Training Operator Python SDK is generated successfully to folder ${SDK_OUTPUT_PATH}/."
rm /tmp/swagger
52 changes: 34 additions & 18 deletions hack/python-sdk/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,26 +23,48 @@ import (
"strings"

"github.com/go-openapi/spec"
mxJob "github.com/kubeflow/tf-operator/pkg/apis/mxnet/v1"
pytorchJob "github.com/kubeflow/tf-operator/pkg/apis/pytorch/v1"
tfjob "github.com/kubeflow/tf-operator/pkg/apis/tensorflow/v1"
xgboostJob "github.com/kubeflow/tf-operator/pkg/apis/xgboost/v1"
"k8s.io/klog"
"k8s.io/kube-openapi/pkg/common"
)

// Generate OpenAPI spec definitions for TFJob Resource
func main() {
if len(os.Args) <= 1 {
klog.Fatal("Supply a version")
if len(os.Args) <= 2 {
klog.Fatal("Supply a framework and version")
}
version := os.Args[1]
framework := os.Args[1]
version := os.Args[2]
if !strings.HasPrefix(version, "v") {
version = "v" + version
}
oAPIDefs := tfjob.GetOpenAPIDefinitions(func(name string) spec.Ref {
return spec.MustCreateRef("#/definitions/" + common.EscapeJsonPointer(swaggify(name)))
})
Comment on lines -40 to -42
Copy link
Member

Choose a reason for hiding this comment

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

@Jeffwan We might need to generate OpenAPI Definition for our APIs to avoid corev1 definition as you mentioned here: #1389 (comment).
Then, our Swagger will have only Training related APIs.
What do you think ?

Copy link
Member Author

@Jeffwan Jeffwan Aug 30, 2021

Choose a reason for hiding this comment

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

@andreyvelich

Yeah. I think this is the problem. Not sure what corev1 are populated. Do you have any ideas? Seems something is misconfigured in this hack/python-sdk/main.go?

Copy link
Member

Choose a reason for hiding this comment

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

Do we need to use code-generator to generate it ? Similar to this: https://github.com/kubeflow/katib/blob/master/hack/update-openapigen.sh#L49-L53.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. Same tool. I update the --input-dirs to exclude those unrelated files. Thanks @andreyvelich

var oAPIDefs map[string]common.OpenAPIDefinition

switch framework {
case "tensorflow":
oAPIDefs = tfjob.GetOpenAPIDefinitions(func(name string) spec.Ref {
return spec.MustCreateRef("#/definitions/" + common.EscapeJsonPointer(swaggify(name, framework)))
})
case "pytorch":
oAPIDefs = pytorchJob.GetOpenAPIDefinitions(func(name string) spec.Ref {
return spec.MustCreateRef("#/definitions/" + common.EscapeJsonPointer(swaggify(name, framework)))
})
case "mxnet":
oAPIDefs = mxJob.GetOpenAPIDefinitions(func(name string) spec.Ref {
return spec.MustCreateRef("#/definitions/" + common.EscapeJsonPointer(swaggify(name, framework)))
})
case "xgboost":
oAPIDefs = xgboostJob.GetOpenAPIDefinitions(func(name string) spec.Ref {
return spec.MustCreateRef("#/definitions/" + common.EscapeJsonPointer(swaggify(name, framework)))
})
}
Comment on lines +46 to +63
Copy link
Member

Choose a reason for hiding this comment

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

@kubeflow/wg-training-leads Do we want to have to have multiple or one Swagger for each framework?

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean compress all swagger into a big one?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I am just thinking what is the best way to locate these swaggers.

Copy link
Member Author

@Jeffwan Jeffwan Sep 6, 2021

Choose a reason for hiding this comment

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

We still need to generate each for different framework. Then I mixin into one and use it to generate SDK.


defs := spec.Definitions{}
for defName, val := range oAPIDefs {
defs[swaggify(defName)] = val.Schema
defs[swaggify(defName, framework)] = val.Schema
}
swagger := spec.Swagger{
SwaggerProps: spec.SwaggerProps{
Expand All @@ -51,8 +73,8 @@ func main() {
Paths: &spec.Paths{Paths: map[string]spec.PathItem{}},
Info: &spec.Info{
InfoProps: spec.InfoProps{
Title: "tfjob",
Description: "Python SDK for TF-Operator",
Title: framework,
Description: fmt.Sprintf("Python SDK for %v", framework),
Version: version,
},
},
Expand All @@ -65,15 +87,9 @@ func main() {
fmt.Println(string(jsonBytes))
}

func swaggify(name string) string {
name = strings.Replace(name, "github.com/kubeflow/tf-operator/pkg/apis/tensorflow/", "", -1)
name = strings.Replace(name, "github.com/kubeflow/common/job_controller/api/", "", -1)
name = strings.Replace(name, "github.com/kubernetes-sigs/kube-batch/pkg/client/clientset/", "", -1)
name = strings.Replace(name, "k8s.io/api/core/", "", -1)
name = strings.Replace(name, "k8s.io/apimachinery/pkg/apis/meta/", "", -1)
name = strings.Replace(name, "k8s.io/kubernetes/pkg/controller/", "", -1)
name = strings.Replace(name, "k8s.io/client-go/listers/core/", "", -1)
name = strings.Replace(name, "k8s.io/client-go/util/workqueue", "", -1)
func swaggify(name, framework string) string {
name = strings.Replace(name, fmt.Sprintf("github.com/kubeflow/tf-operator/pkg/apis/%s/", framework), "", -1)
name = strings.Replace(name, "github.com/kubeflow/common/pkg/apis/common/", "", -1)
name = strings.Replace(name, "/", ".", -1)
return name
}
Loading