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

bump github.com/prometheus/client_golang to v1.20.5 #5040

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

pfi79
Copy link
Contributor

@pfi79 pfi79 commented Oct 21, 2024

Please note the change in the .dockerignore.

@pfi79 pfi79 requested a review from a team as a code owner October 21, 2024 15:21
@denyeart
Copy link
Contributor

This does appear to fix the image build problem. But I don't know why...can you explain? Why was the version of github.com/prometheus/client_golang from the vendor directory not working?

The default is -mod=vendor, which I think is what we want so that the exact vendor directory is used. The point of checking in the vendor directory is so that it will be used and the build will work even if a dependency gets deleted from github and the go ecosystem. Using -mod=readonly seems like a change in behavior that we did not intend and we could be exposed to dependencies that go away.

From the go doc:

-mod=readonly tells the go command to ignore the vendor directory and to report an error if go.mod needs to be updated.
-mod=vendor tells the go command to use the vendor directory. In this mode, the go command will not use the network or the module cache.

@pfi79
Copy link
Contributor Author

pfi79 commented Oct 21, 2024

This does appear to fix the image build problem. But I don't know why...can you explain? Why was the version of github.com/prometheus/client_golang from the vendor directory not working?

The default is -mod=vendor, which I think is what we want so that the exact vendor directory is used. The point of checking in the vendor directory is so that it will be used and the build will work even if a dependency gets deleted from github and the go ecosystem. Using -mod=readonly seems like a change in behavior that we did not intend and we could be exposed to dependencies that go away.

From the go doc:

-mod=readonly tells the go command to ignore the vendor directory and to report an error if go.mod needs to be updated.
-mod=vendor tells the go command to use the vendor directory. In this mode, the go command will not use the network or the module cache.

I don't know, I'll put the pr in draft.
I'll be thinking and looking.

@pfi79 pfi79 marked this pull request as draft October 21, 2024 18:46
@pfi79 pfi79 force-pushed the bump-client_golang branch from 707f1d6 to d53243d Compare October 22, 2024 13:32
@pfi79 pfi79 marked this pull request as ready for review October 22, 2024 13:33
@pfi79 pfi79 force-pushed the bump-client_golang branch from d53243d to 2c018e9 Compare October 22, 2024 13:35
@pfi79
Copy link
Contributor Author

pfi79 commented Oct 22, 2024

This does appear to fix the image build problem. But I don't know why...can you explain? Why was the version of github.com/prometheus/client_golang from the vendor directory not working?

The default is -mod=vendor, which I think is what we want so that the exact vendor directory is used. The point of checking in the vendor directory is so that it will be used and the build will work even if a dependency gets deleted from github and the go ecosystem. Using -mod=readonly seems like a change in behavior that we did not intend and we could be exposed to dependencies that go away.

From the go doc:

-mod=readonly tells the go command to ignore the vendor directory and to report an error if go.mod needs to be updated.
-mod=vendor tells the go command to use the vendor directory. In this mode, the go command will not use the network or the module cache.

the error was here - .dockerignore

@denyeart
Copy link
Contributor

This does appear to fix the image build problem. But I don't know why...can you explain? Why was the version of github.com/prometheus/client_golang from the vendor directory not working?
The default is -mod=vendor, which I think is what we want so that the exact vendor directory is used. The point of checking in the vendor directory is so that it will be used and the build will work even if a dependency gets deleted from github and the go ecosystem. Using -mod=readonly seems like a change in behavior that we did not intend and we could be exposed to dependencies that go away.
From the go doc:

-mod=readonly tells the go command to ignore the vendor directory and to report an error if go.mod needs to be updated.
-mod=vendor tells the go command to use the vendor directory. In this mode, the go command will not use the network or the module cache.

the error was here - .dockerignore

Ah yes... I see how *test was the problem since it matches latest, good find!

I think you don't need the two file renames anymore, since they don't match *_test, right?

Signed-off-by: Fedor Partanskiy <fredprtnsk@gmail.com>
@pfi79 pfi79 force-pushed the bump-client_golang branch from 2c018e9 to 2c77185 Compare October 22, 2024 14:56
@@ -4,7 +4,7 @@
# SPDX-License-Identifier: Apache-2.0
#

**/*test.go
**/*_test.go
Copy link
Contributor

Choose a reason for hiding this comment

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

nice, find ;)

@C0rWin
Copy link
Contributor

C0rWin commented Oct 22, 2024

the change indeed solves the issue, while since @denyeart had concerns, will wait for his decision.

@denyeart denyeart merged commit 507e676 into hyperledger:main Oct 22, 2024
15 checks passed
@pfi79 pfi79 deleted the bump-client_golang branch October 22, 2024 17:56
denyeart added a commit to denyeart/fabric that referenced this pull request Nov 24, 2024
backport of hyperledger#5040

Signed-off-by: David Enyeart <enyeart@us.ibm.com>
C0rWin pushed a commit that referenced this pull request Nov 25, 2024
)

backport of #5040

Signed-off-by: David Enyeart <enyeart@us.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants