HMS-9066: Updating go version to 1.24 by mayurilahane · Pull Request #21 · content-services/tang

mayurilahane · 2025-09-18T04:32:26Z

No description provided.

rverdile

I would do a dependency bump here: go get -u ./...

mayurilahane · 2025-09-24T15:45:05Z

Getting lint errors which suggest to use the pattern to set verison is 1.24.x

and i have set it to 1.24 which seems a better option to me

rverdile · 2025-09-24T17:04:54Z

It's a little confusing, but the linter errors are actually caused by this line here: https://github.com/content-services/tang/blob/main/.github/workflows/tang-actions.yaml#L23

It needs to be updated to a newer version of golangci-lint. I would say v2.4 to match the backend: https://github.com/content-services/content-sources-backend/blob/main/.github/workflows/content-sources-actions.yml#L51-L53

mayurilahane · 2025-09-24T18:56:18Z

It's a little confusing, but the linter errors are actually caused by this line here: https://github.com/content-services/tang/blob/main/.github/workflows/tang-actions.yaml#L23

It needs to be updated to a newer version of golangci-lint. I would say v2.4 to match the backend: https://github.com/content-services/content-sources-backend/blob/main/.github/workflows/content-sources-actions.yml#L51-L53

how do you figure it out ? that it should match with backend golangci-lint version ?
i thought it will update by itself after go get -u cmd

rverdile · 2025-09-24T19:04:15Z

how do you figure it out ? that it should match with backend golangci-lint version ?
i thought it will update by itself after go get -u cmd

Good question. go get -u ./... will only upgrade the dependencies imported in the go.mod. golangci-lint is not a dependency, but a CLI tool for linting. We install and use it locally with make lint. so everything probably worked locally. However, the golangci-lint action is a separate github action, independent of what we use locally. We should maybe make some change to the action to match what we use locally, but just bumping the version is good for now :)

rverdile · 2025-09-24T19:43:38Z

there's a CI error. sounds like you just need to bump the action version

Failed to run: Error: invalid version string 'v2.4.0', golangci-lint v2 is not supported by golangci-lint-action v6, you must update to golangci-lint-action v7., Error: invalid version string 'v2.4.0', golangci-lint v2 is not supported by golangci-lint-action v6, you must update to golangci-lint-action v7

mayurilahane · 2025-09-25T21:07:21Z

/retest

…ling - Bump Go version to 1.24 and golangci-lint to v2.4.0 in GitHub Actions workflow - Update .golangci.yaml for compatibility with golangci-lint v2 - Refactor internal/zestwrapper/rpm.go to use safer HTTP response body closing with error handling and improved logging - Switch to math/rand/v2 in pkg/tangy/queries.go for randomness, remove golang.org/x/exp/rand (deprecated) dependency - Update dependancies in go.mod and go.sum

marusak · 2025-09-30T12:27:45Z

Bump Go version to 1.24 and golangci-lint to v2.4.0 in GitHub Actions workflow

Update .golangci.yaml for compatibility with golangci-lint v2

Refactor internal/zestwrapper/rpm.go to use safer HTTP response body closing with error handling and improved logging

Switch to math/rand/v2 in pkg/tangy/queries.go for randomness, remove golang.org/x/exp/rand (deprecated) dependency

Update dependancies in go.mod and go.sum

There seem to be ~4 different changes? But they are all in the same commit 🤔 Are they strictly connected thus being in one commit?

rverdile · 2025-09-30T13:10:13Z

internal/zestwrapper/rpm.go

+	defer func() {
+		if closeErr := httpResp.Body.Close(); closeErr != nil {
+			fmt.Printf("failed to close response body: %v\n", closeErr)
+		}
+	}()


We prefer to ignore this linter error. Can you update the PR to use the config from the backend? That will make sure all the right linters are turned on

rverdile · 2025-09-30T13:11:21Z

There seem to be ~4 different changes? But they are all in the same commit 🤔 Are they strictly connected thus being in one commit?

@marusak They're all connected to bumping the go version and failures related to that

mayurilahane · 2025-09-30T13:25:14Z

Bump Go version to 1.24 and golangci-lint to v2.4.0 in GitHub Actions workflow

Update .golangci.yaml for compatibility with golangci-lint v2

Refactor internal/zestwrapper/rpm.go to use safer HTTP response body closing with error handling and improved logging

Switch to math/rand/v2 in pkg/tangy/queries.go for randomness, remove golang.org/x/exp/rand (deprecated) dependency

Update dependancies in go.mod and go.sum

There seem to be ~4 different changes? But they are all in the same commit 🤔 Are they strictly connected thus being in one commit?

Hey yes, these are connected.
I updated go and go linter version.

Updating linter version, recomended these changes which looked good to me in pkg/tangy/queries.go and internal/zestwrapper/rpm.go.

Although i need to undo the changes in internal/zestwrapper/rpm.go as per
#21 (comment)

Update .golangci.yaml to match backend linter configuration to ensure all appropriate linters are enabled. - G115 - Integer overflow in pool limit: Change PoolLimit type from int to int32 to match pgxpool.Config.MaxConns, eliminating type conversion and preventing potential integer overflow. - ST1005 - Error strings should not be capitalized Fix error message capitalization in zestwrapper/rpm.go

rverdile · 2025-10-01T16:14:34Z

pkg/tangy/interface.go

 		dbConfig.PoolLimit = DefaultMaxPoolLimit
 	}
-	pxConfig.MaxConns = int32(dbConfig.PoolLimit)
+	pxConfig.MaxConns = dbConfig.PoolLimit


This works! but changing types here will also require changes in the backend. Let's fix this linting issue by checking the int max, instead of changing the type of dbConfig.PoolLimit to int32, like this: https://github.com/content-services/content-sources-backend/blob/93863b503e71c6bb93e10f1b2239887efc476c86/pkg/tasks/queue/pgqueue.go#L218-L220

Validate that dbConfig.PoolLimit is within 32-bit integer range (math.MinInt32 to math.MaxInt32) before converting to int32 to prevent overflow issues.

rverdile

looks good! going to merge and release a new version. Then you'll be able to update your backend PR

rverdile reviewed Sep 24, 2025

View reviewed changes

mayurilahane force-pushed the mlahane/HMS-9066 branch 6 times, most recently from 63acd74 to ef793b0 Compare September 25, 2025 20:37

mayurilahane force-pushed the mlahane/HMS-9066 branch 4 times, most recently from 0f5492c to 4127eb5 Compare September 29, 2025 05:20

mayurilahane force-pushed the mlahane/HMS-9066 branch from 4127eb5 to 5127acc Compare September 29, 2025 05:33

mayurilahane marked this pull request as ready for review September 29, 2025 05:36

rverdile reviewed Sep 30, 2025

View reviewed changes

mayurilahane force-pushed the mlahane/HMS-9066 branch from 19cef2a to 8063453 Compare October 1, 2025 02:51

mayurilahane force-pushed the mlahane/HMS-9066 branch from 8063453 to 115be3e Compare October 1, 2025 03:09

rverdile reviewed Oct 1, 2025

View reviewed changes

rverdile self-assigned this Oct 2, 2025

add: validation for pool limit bounds in New function

132ccfb

Validate that dbConfig.PoolLimit is within 32-bit integer range (math.MinInt32 to math.MaxInt32) before converting to int32 to prevent overflow issues.

rverdile approved these changes Oct 7, 2025

View reviewed changes

rverdile merged commit 08a4021 into content-services:main Oct 7, 2025
2 checks passed

Conversation

mayurilahane commented Sep 18, 2025

Uh oh!

rverdile left a comment

Choose a reason for hiding this comment

Uh oh!

mayurilahane commented Sep 24, 2025

Uh oh!

rverdile commented Sep 24, 2025

Uh oh!

mayurilahane commented Sep 24, 2025

Uh oh!

rverdile commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rverdile commented Sep 24, 2025

Uh oh!

mayurilahane commented Sep 25, 2025

Uh oh!

marusak commented Sep 30, 2025

Uh oh!

rverdile Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

rverdile commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mayurilahane commented Sep 30, 2025

Uh oh!

rverdile Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rverdile left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rverdile commented Sep 24, 2025 •

edited

Loading

rverdile commented Sep 30, 2025 •

edited

Loading

rverdile Oct 1, 2025 •

edited

Loading