Update links and files with the new URL #1434

andreyvelich · 2021-10-06T12:06:19Z

Since we updated the repository name: #1348, we should keep our links up to date.

Also, I updated Go version to 1.17 from 1.14.

Please take a look.

/assign @kubeflow/wg-training-leads

andreyvelich · 2021-10-06T12:19:42Z

@alembiewski After running ./hack/python-sdk.sh the description for Swagger which has all APIs was modified: https://github.com/kubeflow/training-operator/blob/2fe42711fee8958930a3958fcf1ff2c508d90113/hack/python-sdk/swagger.json.
Any ideas how to keep this file consistent with the name Python SDK for Kubeflow Training ?

Maybe we should modify our gen-sdk script to have only 1 Swagger for all frameworks:

training-operator/hack/python-sdk/gen-sdk.sh

Lines 42 to 46 in 2fe4271

    
           for FRAMEWORK in ${FRAMEWORKS[@]}; do 
        
             SWAGGER_CODEGEN_FILE="pkg/apis/${FRAMEWORK}/v1/swagger.json" 
        
             echo "Generating swagger file for ${FRAMEWORK} ..." 
        
             go run "${repo_root}"/hack/python-sdk/main.go "${FRAMEWORK}" ${VERSION} >"${SWAGGER_CODEGEN_FILE}" 
        
           done

?

What do you think @alembiewski @Jeffwan ?

terrytangyuan

/lgtm
/approve

google-oss-robot · 2021-10-06T12:21:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreyvelich, 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~~ [terrytangyuan]

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

terrytangyuan · 2021-10-06T12:22:09Z

/hold

alembiewski · 2021-10-06T13:55:33Z

@andreyvelich, thanks for catching this!
raised a quick PR with a fix: #1435

Maybe we should modify our gen-sdk script to have only 1 Swagger for all frameworks

yeah, that's what we actually do with swagger mixin later in this script, but the problem is that the first spec (tensorflow in our case) is used as primary, so that's why we have incorrect metadata in the resulting spec.

andreyvelich · 2021-10-06T14:23:48Z

Thanks @alembiewski.
From my point of view, we can just keep one swagger for all APIs.
In that case, we don't need swagger mixin and generation will be simplified.

alembiewski · 2021-10-06T20:47:55Z

@andreyvelich, I got your point and I agree with the proposed approach. Let me take a closer look at how we can simplify that. Closing #1435.

Jeffwan · 2021-10-07T01:33:33Z

.travis.yml

@@ -3,8 +3,9 @@ language: go
 go:
  - "1.13"
  - "1.14"
+  - "1.17"


what's the strategy here? It prefer higher version?

I think it was used to test various go env.
We will remove Travis in this PR: kubeflow/testing#967, so I can remove this change.

tenzen-y · 2021-10-08T05:59:19Z

Should we rename the following description in developer_guide.md? @andreyvelich

# Developer Guide

Tf-operator is currently at v1.

andreyvelich · 2021-10-08T11:35:43Z

Should we rename the following description in developer_guide.md? @andreyvelich
# Developer Guide

Tf-operator is currently at v1.

Sure, changed.

andreyvelich · 2021-10-08T11:37:11Z

Great news, the CI has been triggered after this PR: kubeflow/testing#967.
cc @kubeflow/wg-training-leads

Jeffwan · 2021-10-08T18:30:53Z

@andreyvelich #1440 is merged. Let's rebase the change?

tenzen-y · 2021-10-08T18:49:00Z

Sorry, I forgot to fix the coveralls link of Readme in #1440. However, it seems to be correctly described in this PR, so please give priority to this PR change when rebasing. @andreyvelich

[![Coverage Status](https://coveralls.io/repos/github/kubeflow/training-operator/badge.svg?branch=master)](https://coveralls.io/github/kubeflow/tf-operator?branch=master)

andreyvelich · 2021-10-08T23:45:24Z

@Jeffwan @tenzen-y Thank you for the updates!
I rebased this PR, please take a look at the changes.

coveralls · 2021-10-08T23:47:36Z

Pull Request Test Coverage Report for Build 1323699138

0 of 40 (0.0%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 8.161%

Changes Missing Coverage	Changed/Added Lines	%
pkg/apis/tensorflow/v1/openapi_generated.go	13	0.0%
pkg/apis/xgboost/v1/openapi_generated.go	13	0.0%
pkg/apis/mxnet/v1/openapi_generated.go	14	0.0%

Totals
Change from base Build 1322574458:	0.0%
Covered Lines:	311
Relevant Lines:	3811

💛 - Coveralls

Jeffwan · 2021-10-09T02:47:38Z

README.md

@@ -91,19 +92,19 @@ Please refer to [CHANGELOG](CHANGELOG.md)

 The following table lists the most recent few versions of the operator.

-| Operator Version | API Version | Kubernetes Version |


Did you use some format tools on this kind of format issue?

Yes, I am using Prettier to automatically format these docs.
Does it sounds good to you ?

Sounds good. I am thinking we should force to use same tool and probably add it to CI later.

docs/monitoring/README.md

Jeffwan · 2021-10-09T03:00:26Z

em.. sorry @andreyvelich I merge the other PR and we need to rebase this one one more time.

tenzen-y · 2021-10-09T05:24:23Z

LGTM👍

Use Go 1.17

andreyvelich · 2021-10-09T14:28:33Z

I made the changes.
@alembiewski @Jeffwan @tenzen-y Please take a look.

tenzen-y · 2021-10-09T15:24:39Z

/lgtm
LGTM!
If it looks good to you, please comment with /unhold. @andreyvelich @Jeffwan

Jeffwan · 2021-10-09T20:49:44Z

/unhold

Jeffwan · 2021-10-09T20:49:50Z

Great! Thanks!

google-oss-robot added do-not-merge/work-in-progress size/XL labels Oct 6, 2021

google-oss-robot requested review from jinchihe and terrytangyuan October 6, 2021 12:06

terrytangyuan approved these changes Oct 6, 2021

View reviewed changes

google-oss-robot assigned terrytangyuan Oct 6, 2021

google-oss-robot added the lgtm label Oct 6, 2021

google-oss-robot added the approved label Oct 6, 2021

google-oss-robot added the do-not-merge/hold label Oct 6, 2021

Jeffwan reviewed Oct 7, 2021

View reviewed changes

andreyvelich mentioned this pull request Oct 7, 2021

Add CI to run the tests for Go #1440

Merged

google-oss-robot removed the lgtm label Oct 7, 2021

andreyvelich changed the title ~~[WIP] Update links and files with the new URL~~ Update links and files with the new URL Oct 8, 2021

google-oss-robot removed the do-not-merge/work-in-progress label Oct 8, 2021

andreyvelich force-pushed the fix-tf-operator-link branch from 4047bd7 to 9380ad8 Compare October 8, 2021 23:42

andreyvelich mentioned this pull request Oct 9, 2021

Display coverage % in GitHub actions list #1442

Closed

Jeffwan reviewed Oct 9, 2021

View reviewed changes

andreyvelich added 6 commits October 9, 2021 15:10

Update links and corresponding files for training-operator repository

0b7c1a4

Use Go 1.17

Generate SDK

6ed9b09

Remove Travis

aff0508

Modify developer guide

f5c5e43

Fix go mod

fdc86d1

Fix legacy docs

835eb4d

andreyvelich force-pushed the fix-tf-operator-link branch from 105d76c to 835eb4d Compare October 9, 2021 14:11

andreyvelich added 3 commits October 9, 2021 15:13

Generate SDK

f6909b7

Remove travis

f413823

Update ignore files

36a6e7e

google-oss-robot assigned tenzen-y Oct 9, 2021

google-oss-robot added the lgtm label Oct 9, 2021

google-oss-robot removed the do-not-merge/hold label Oct 9, 2021

google-oss-robot merged commit 4b5d651 into kubeflow:master Oct 9, 2021

andreyvelich deleted the fix-tf-operator-link branch October 9, 2021 21:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update links and files with the new URL #1434

Update links and files with the new URL #1434

andreyvelich commented Oct 6, 2021

andreyvelich commented Oct 6, 2021

terrytangyuan left a comment

google-oss-robot commented Oct 6, 2021

terrytangyuan commented Oct 6, 2021

alembiewski commented Oct 6, 2021 •

edited

Loading

andreyvelich commented Oct 6, 2021

alembiewski commented Oct 6, 2021

Jeffwan Oct 7, 2021

andreyvelich Oct 7, 2021

tenzen-y commented Oct 8, 2021

andreyvelich commented Oct 8, 2021

andreyvelich commented Oct 8, 2021

Jeffwan commented Oct 8, 2021

tenzen-y commented Oct 8, 2021 •

edited

Loading

andreyvelich commented Oct 8, 2021

coveralls commented Oct 8, 2021 •

edited

Loading

Jeffwan Oct 9, 2021

andreyvelich Oct 9, 2021

Jeffwan Oct 9, 2021

Jeffwan commented Oct 9, 2021 •

edited

Loading

tenzen-y commented Oct 9, 2021 •

edited

Loading

andreyvelich commented Oct 9, 2021

tenzen-y commented Oct 9, 2021 •

edited

Loading

Jeffwan commented Oct 9, 2021

Jeffwan commented Oct 9, 2021

@@ @@ -3,8 +3,9 @@ language: go @@
               go:
                 - "1.13"
                 - "1.14"
+                - "1.17"

		@@ -91,19 +92,19 @@ Please refer to [CHANGELOG](CHANGELOG.md)

		The following table lists the most recent few versions of the operator.

		\| Operator Version \| API Version \| Kubernetes Version \|

Update links and files with the new URL #1434

Update links and files with the new URL #1434

Conversation

andreyvelich commented Oct 6, 2021

andreyvelich commented Oct 6, 2021

terrytangyuan left a comment

Choose a reason for hiding this comment

google-oss-robot commented Oct 6, 2021

terrytangyuan commented Oct 6, 2021

alembiewski commented Oct 6, 2021 • edited Loading

andreyvelich commented Oct 6, 2021

alembiewski commented Oct 6, 2021

Jeffwan Oct 7, 2021

Choose a reason for hiding this comment

andreyvelich Oct 7, 2021

Choose a reason for hiding this comment

tenzen-y commented Oct 8, 2021

andreyvelich commented Oct 8, 2021

andreyvelich commented Oct 8, 2021

Jeffwan commented Oct 8, 2021

tenzen-y commented Oct 8, 2021 • edited Loading

andreyvelich commented Oct 8, 2021

coveralls commented Oct 8, 2021 • edited Loading

Pull Request Test Coverage Report for Build 1323699138

💛 - Coveralls

Jeffwan Oct 9, 2021

Choose a reason for hiding this comment

andreyvelich Oct 9, 2021

Choose a reason for hiding this comment

Jeffwan Oct 9, 2021

Choose a reason for hiding this comment

Jeffwan commented Oct 9, 2021 • edited Loading

tenzen-y commented Oct 9, 2021 • edited Loading

andreyvelich commented Oct 9, 2021

tenzen-y commented Oct 9, 2021 • edited Loading

Jeffwan commented Oct 9, 2021

Jeffwan commented Oct 9, 2021

alembiewski commented Oct 6, 2021 •

edited

Loading

tenzen-y commented Oct 8, 2021 •

edited

Loading

coveralls commented Oct 8, 2021 •

edited

Loading

Jeffwan commented Oct 9, 2021 •

edited

Loading

tenzen-y commented Oct 9, 2021 •

edited

Loading

tenzen-y commented Oct 9, 2021 •

edited

Loading