chore: Upgrade Golang version from 1.17 to 1.19 #68

spolti · 2023-11-01T19:02:38Z

chore: Go lang 1.17 is a little bit old and 1.19 as well. However, before taking a greater step to higher version would might be better to break it in smaller changes since the amount of changes that this update has required is a little bit high, here is a small description of what was changed by this update:

Outdated linters:
- WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
- WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
- WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
Deprecation of the io/ioutils package:
- SA1019: "io/ioutil" has been deprecated since Go 1.19: As of Go 1.16, the same functionality is now provided by package [io] or package [os] - Used the os package and the needed code changes applied
At puller/config.go
- ( undefined: GetEnvString)
- Fixed by import . "github.com/kserve/modelmesh-runtime-adapter/internal/envconfig"
Some comments and headers adjusted
small identation changes made as part of go fmt.

Fixes #64

chore: Go lang 1.17 and a little bit old and 1.19 as well. However, before taking a greater step to higher version would might be better to break it in smaller changes since the amount of changes that this update has required is a little bit high, here is a small description of what was changed by this update: - Outdated linters: - WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. - WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. - WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. - Deprecation of the io/ioutils package: - SA1019: "io/ioutil" has been deprecated since Go 1.19: As of Go 1.16, the same functionality is now provided by package [io] or package [os] - Used the `os` package and the needed code changes applied - At puller/config.go - ( undefined: GetEnvString) - Fixed by `import . "github.com/kserve/modelmesh-runtime-adapter/internal/envconfig"` - Some comments and headers adjusted - small identation changes made as part of go fmt. Fixes kserve#68 Signed-off-by: Spolti <[email protected]>

Jooho · 2023-11-09T15:55:39Z

@ckadner could you please review this PR?

ckadner

Thanks @spolti for going through the tedious process of updating all the code! I just have a few questions/suggestions.

Makefile

internal/util/fileutil.go

model-mesh-mlserver-adapter/server/server.go

model-mesh-ovms-adapter/server/modelmanager.go

scripts/fmt.sh

spolti · 2023-11-15T17:11:20Z

Hi @ckadner, it is ready for re-review :)

chore: Go lang 1.17 and a little bit old and 1.19 as well. However, before taking a greater step to higher version would might be better to break it in smaller changes since the amount of changes that this update has required is a little bit high, here is a small description of what was changed by this update: - Outdated linters: - WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. - WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. - WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. - Deprecation of the io/ioutils package: - SA1019: "io/ioutil" has been deprecated since Go 1.19: As of Go 1.16, the same functionality is now provided by package [io] or package [os] - Used the `os` package and the needed code changes applied - At puller/config.go - ( undefined: GetEnvString) - Fixed by `import . "github.com/kserve/modelmesh-runtime-adapter/internal/envconfig"` - Some comments and headers adjusted - small identation changes made as part of go fmt. Fixes kserve#64 Signed-off-by: Spolti <[email protected]>

Signed-off-by: Spolti <[email protected]>

ckadner · 2023-11-23T02:03:10Z

Hi @ckadner, it is ready for re-review :)

thanks @spolti -- I added 2 more comments to the threads from my previous review

ckadner

Thanks @spolti -- 2 follow-ups from my previous review

scripts/fmt.sh

internal/envconfig/envconfig.go

kserve-oss-bot · 2023-11-23T19:31:09Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: spolti
To complete the pull request process, please assign rafvasq after the PR has been reviewed.
You can assign the PR to them by writing /assign @rafvasq in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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

rafvasq

Tested with quickstart, no issues! FYI @ckadner

Signed-off-by: Spolti <[email protected]>

spolti · 2023-11-23T20:31:01Z

Tested with quickstart, no issues! FYI @ckadner

Hi @rafvasq, thanks for testing.
Do you mind sharing about this tool you used, quickstart, is this a public tool, or?

rafvasq · 2023-11-23T20:48:43Z

Tested with quickstart, no issues! FYI @ckadner

Hi @rafvasq, thanks for testing. Do you mind sharing about this tool you used, quickstart, is this a public tool, or?

@spolti, simply meant that I deployed MM with your updates to this component and deployed/inferenced an inference service as outlined in the quickstart guide :)

ckadner

So close! :-)

.pre-commit-config.yaml

internal/modelschema/modelschema.go

scripts/fmt.sh

spolti · 2023-11-28T16:55:51Z

service as outlined in the quickstart guide :)

Oh, right, quickstart :)

Signed-off-by: Spolti <[email protected]>

internal/envconfig/envconfig.go

ckadner

Thanks @spolti -- just need some minor cleanup.

.gitignore

internal/envconfig/envconfig.go

internal/util/fileutil.go

Signed-off-by: Spolti <[email protected]>

ckadner

Hi @spolti -- okay, I understand that either go fmt will replace the spaces with tabs even in files that don't necessarily needed changing, or we/you add the empty line above the package declaration in all *.go files. It's not worth another round of review. Let's do a sweep of all files after this PR.

/lgtm

internal/modelschema/modelschema.go

oss-prow-bot · 2023-11-30T01:03:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ckadner, spolti

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~~ [ckadner]

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

ckadner · 2023-11-30T01:20:42Z

Thank you @spolti !

spolti · 2023-11-30T17:37:32Z

Hi @spolti -- okay, I understand that either go fmt will replace the spaces with tabs even in files that don't necessarily needed changing, or we/you add the empty line above the package declaration in all *.go files. It's not worth another round of review. Let's do a sweep of all files after this PR.

/lgtm

Hi @ckadner, I've created this issue to track the headers change: #75

spolti · 2023-11-30T18:12:16Z

Actually you've opened already, closed mine and add more information on #74

- Remove the linters for "deadcode", "structcheck", "varcheck" - Use "os" packages instead of deprecated "io/ioutil" (SA1019) - Capture pre-commit output in a local log file Fixes opendatahub-io#64 --------- Signed-off-by: Spolti <[email protected]>

kserve-oss-bot requested review from ckadner and joerunde November 1, 2023 19:02

spolti force-pushed the issue64 branch 3 times, most recently from 1b40deb to d2167da Compare November 1, 2023 19:48

ckadner reviewed Nov 10, 2023

View reviewed changes

spolti force-pushed the issue64 branch 3 times, most recently from a2b7326 to 70ce114 Compare November 15, 2023 17:00

spolti added 2 commits November 21, 2023 16:28

review suggestions

e0e001d

Signed-off-by: Spolti <[email protected]>

spolti force-pushed the issue64 branch from 70ce114 to e0e001d Compare November 21, 2023 19:30

rafvasq self-requested a review November 22, 2023 20:43

ckadner requested review from ckadner, tjohnson31415 and njhill and removed request for joerunde November 22, 2023 22:40

ckadner mentioned this pull request Nov 23, 2023

Update Go to supported version (1.20) kserve/modelmesh-serving#452

Closed

ckadner changed the title ~~bump go version from 1.17 to 1.19~~ chore: Upgrade Golang version from 1.17 to 1.19 Nov 23, 2023

kserve deleted a comment from kserve-oss-bot Nov 23, 2023

ckadner requested changes Nov 23, 2023

View reviewed changes

scripts/fmt.sh Outdated Show resolved Hide resolved

internal/envconfig/envconfig.go Outdated Show resolved Hide resolved

ckadner added this to the v0.11.2 milestone Nov 23, 2023

rafvasq reviewed Nov 23, 2023

View reviewed changes

apply suggestions

c1280e5

Signed-off-by: Spolti <[email protected]>

rafvasq mentioned this pull request Nov 23, 2023

chore: Upgrade Golang version from 1.18 to 1.19 kserve/rest-proxy#28

Merged

ckadner requested changes Nov 23, 2023

View reviewed changes

.pre-commit-config.yaml Outdated Show resolved Hide resolved

internal/modelschema/modelschema.go Show resolved Hide resolved

scripts/fmt.sh Outdated Show resolved Hide resolved

spolti mentioned this pull request Nov 27, 2023

Address High vulnerabilities in ModelMesh SNYK reports opendatahub-io/modelmesh-serving#249

Closed

8 tasks

spolti force-pushed the issue64 branch from 9cd27d4 to 0fde796 Compare November 28, 2023 17:18

remove tab from license and add new line after it

cd9b8dd

Signed-off-by: Spolti <[email protected]>

spolti force-pushed the issue64 branch from 0fde796 to cd9b8dd Compare November 28, 2023 17:19

spolti commented Nov 28, 2023

View reviewed changes

internal/envconfig/envconfig.go Outdated Show resolved Hide resolved

spolti requested a review from ckadner November 28, 2023 17:23

spolti mentioned this pull request Nov 28, 2023

Address High vulnerabilities in Modelmesh-runtime-adapter SNYK reports opendatahub-io/modelmesh-serving#256

Closed

1 task

spolti force-pushed the issue64 branch from dd53c5f to cad6a8f Compare November 28, 2023 19:03

ckadner requested changes Nov 28, 2023

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

internal/envconfig/envconfig.go Outdated Show resolved Hide resolved

internal/util/fileutil.go Outdated Show resolved Hide resolved

spolti force-pushed the issue64 branch 3 times, most recently from d6e9d80 to cf4e67c Compare November 29, 2023 14:20

rename the pre-commint log file

0348f61

Signed-off-by: Spolti <[email protected]>

spolti force-pushed the issue64 branch from cf4e67c to 0348f61 Compare November 29, 2023 14:26

ckadner approved these changes Nov 30, 2023

View reviewed changes

internal/modelschema/modelschema.go Show resolved Hide resolved

oss-prow-bot bot assigned ckadner Nov 30, 2023

oss-prow-bot bot added the lgtm label Nov 30, 2023

oss-prow-bot bot added the approved label Nov 30, 2023

ckadner mentioned this pull request Nov 30, 2023

Format all *.go files to have the empty line above the package declaration #74

Closed

ckadner merged commit 0ba2f47 into kserve:main Nov 30, 2023
7 checks passed

spolti deleted the issue64 branch November 30, 2023 17:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Upgrade Golang version from 1.17 to 1.19 #68

chore: Upgrade Golang version from 1.17 to 1.19 #68

spolti commented Nov 1, 2023 •

edited

Loading

Jooho commented Nov 9, 2023

ckadner left a comment

spolti commented Nov 15, 2023

ckadner commented Nov 23, 2023 •

edited

Loading

ckadner left a comment

kserve-oss-bot commented Nov 23, 2023

rafvasq left a comment

spolti commented Nov 23, 2023 •

edited

Loading

rafvasq commented Nov 23, 2023 •

edited

Loading

ckadner left a comment

spolti commented Nov 28, 2023

ckadner left a comment

ckadner left a comment

oss-prow-bot bot commented Nov 30, 2023

ckadner commented Nov 30, 2023

spolti commented Nov 30, 2023

spolti commented Nov 30, 2023

chore: Upgrade Golang version from 1.17 to 1.19 #68

chore: Upgrade Golang version from 1.17 to 1.19 #68

Conversation

spolti commented Nov 1, 2023 • edited Loading

Jooho commented Nov 9, 2023

ckadner left a comment

Choose a reason for hiding this comment

spolti commented Nov 15, 2023

ckadner commented Nov 23, 2023 • edited Loading

ckadner left a comment

Choose a reason for hiding this comment

kserve-oss-bot commented Nov 23, 2023

rafvasq left a comment

Choose a reason for hiding this comment

spolti commented Nov 23, 2023 • edited Loading

rafvasq commented Nov 23, 2023 • edited Loading

ckadner left a comment

Choose a reason for hiding this comment

spolti commented Nov 28, 2023

ckadner left a comment

Choose a reason for hiding this comment

ckadner left a comment

Choose a reason for hiding this comment

oss-prow-bot bot commented Nov 30, 2023

ckadner commented Nov 30, 2023

spolti commented Nov 30, 2023

spolti commented Nov 30, 2023

spolti commented Nov 1, 2023 •

edited

Loading

ckadner commented Nov 23, 2023 •

edited

Loading

spolti commented Nov 23, 2023 •

edited

Loading

rafvasq commented Nov 23, 2023 •

edited

Loading