Update cockroach-go module version #146
Conversation
|
This change is |
|
Thanks Rafi, |
|
I've never seen this lint failure before. something must have changed in the CI environment, since we haven't updated this repo in a while only the last few things seem actionable: edit: nvm, i don't see why the linter is complaining. those lines look fine to me. |
go.mod
Outdated
| @@ -3,11 +3,12 @@ module github.com/cockroachdb/examples-orms | |||
| go 1.13 | |||
There was a problem hiding this comment.
perhaps try updating this to go 1.17?
|
Ah, one more shot in the dark! The file in |
|
Ok will try. Any way to run this locally? Will be easier to iterate on this. |
|
The issue is in the Github CI environment, and I don't know how to run that locally. Maybe Github has instructions for it in their docs. |
|
I see. Okay, let me dig into this today. |
.github/workflows/golangci-lint.yml
Outdated
| @@ -21,4 +21,4 @@ jobs: | |||
| uses: golangci/golangci-lint-action@v2 | |||
There was a problem hiding this comment.
I found this repo: https://github.com/golangci/golangci-lint-action
Looks like they have a @v3 now... They have a thing in their README saying how to configure it: https://github.com/golangci/golangci-lint-action#compatibility
(In case you couldn't tell, I have never worked with this Github CI configuration either, so I'm suggesting things ad-hoc)
There was a problem hiding this comment.
Thanks for the help. This is new to me as well so let me read up on it a little. Btw, this test isn't marked as required. Does that mean it would be ok to merge this PR and fix it in a follow up? Ideally, I would like to fix it before merge but wanted to know what my options are in case debugging this takes too long.
There was a problem hiding this comment.
oh, nice catch. in that case, yes i'll approve this and if you can't make any further progress on the linter feel free to merge.
There was a problem hiding this comment.
Interestingly its running a go version of 1.18 even though go mod specifies it as 1.17
GOVERSION="go1.18.2"
I think I need to understand the setup better.
The new cockroach-go module adds a tenant scope argument while generating client certificates for tenant servers within the cockroach go repo. We need this new change in order for the tests with example-orms to work correctly with the upcoming changes for tenant scoped client certificates. Without this change, the tenant tests will fail as the client certificates generated through the cockroach-go modules will be unable to authenticate correctly with the tenant server.
|
Ta-da, managed to get it working, lol after playing around with it locally. Turns out needed a |
|
amazing! thank you for getting into the weeds on it |
There was a problem hiding this comment.
Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @rafiss)
go.mod line 3 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
perhaps try updating this to
go 1.17?
Done.
The new cockroach-go module adds a tenant scope argument
while generating client certificates for tenant servers
within the cockroach go repo. We need this new change in
order for the tests with example-orms to work correctly
with the upcoming changes for tenant scoped client certificates.
Without this change, the tenant tests will fail as the client
certificates generated through the cockroach-go modules will be
unable to authenticate correctly with the tenant server.