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

jenkins: work around git.apache.org failures #2944

Merged
merged 3 commits into from
Sep 4, 2019

Conversation

thepaul
Copy link
Contributor

@thepaul thepaul commented Sep 3, 2019

What:

Adds a replace directive to go.mod which causes Go to use the Apache Thrift module from github instead of the one at git.apache.org (which is no longer reachable).

Why:

I got really tired of the errors with Go being unable to fetch git.apache.org/thrift.git. Apparently, Thrift development officially moved to Github quite some time ago anyway. See:

https://github.com/apache/thrift/commit/9ee29516c419b7eaa95ed89a93b135ea1c683576

So this replace directive causes opencensus.io/foo to use the correct url for the thrift module. Until opencensus.io fixes its dependencies:

https://github.com/census-instrumentation/opencensus-go/issues/1158

this might be the only recourse.

Please describe the tests:

  • If Jenkins can build at all, this functionality is working.

Please describe the performance impact:

Code Review Checklist (to be filled out by reviewer)

  • Does the PR describe what changes are being made?
  • Does the PR describe why the changes are being made?
  • Does the code follow our style guide?
  • Does the code follow our testing guide?
  • Is the PR appropriately sized? (If it could be broken into smaller PRs it should be)
  • Does the new code have enough tests? (every PR should have tests or justification otherwise. Bug-fix PRs especially)
  • Does the new code have enough documentation that answers "how do I use it?" and "what does it do?"? (both source documentation and higher level, diagrams?)
  • Does any documentation need updating?
  • Do the database access patterns make sense?

@thepaul thepaul added Request Code Review Code review requested Reviewer Can Merge If all checks have passed, non-owner can merge PR labels Sep 3, 2019
@thepaul thepaul requested review from zeebo, egonelbre and a team September 3, 2019 23:27
@cla-bot cla-bot bot added the cla-signed label Sep 3, 2019
@ghost ghost removed their request for review September 3, 2019 23:27
zeebo
zeebo previously approved these changes Sep 3, 2019
Copy link
Contributor

@zeebo zeebo left a comment

Choose a reason for hiding this comment

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

  1. Seems like we changed the version of thrift we depend on? Though, it also seems like the source isn't actually part of the build, so it probably doesn't matter.
  2. I think this should also be fixed by the newly released go1.13 because it by default goes through a centralized proxy that caches all this stuff.

Looks fine to me, though.

@thepaul
Copy link
Contributor Author

thepaul commented Sep 3, 2019

I tried to keep the same hash-version in place, but Go insisted on ignoring the directive entirely until I put an actual version there. I don't know why.

At least I didn't make up v0.12.0 myself; it's what other Go users are recommending to each other in a few different places in order to fix this unresolvable go.opencensus.io dependency. See for example willnorris/imageproxy#190 .

So yeah maybe it's not perfect, but since go.opencensus.io only comes in to the picture because of golang-migrate, and we only use golang-migrate in a very limited way, I feel very comfortable in saying we don't actually use any thrift code anywhere in our product, so... yeah. Whatever.

Go 1.13 will be nice

@thepaul
Copy link
Contributor Author

thepaul commented Sep 3, 2019

Wait what did you just say Go 1.13 is released now??

@zeebo
Copy link
Contributor

zeebo commented Sep 3, 2019

Indeed: https://blog.golang.org/go1.13

we can't build the old version because of this same problem, so that
check can't succeed.
@littleskunk
Copy link
Member

Awesome. After updating my storage node to go1.13 it is finally running again. Yea!

zeebo
zeebo previously approved these changes Sep 4, 2019
Jenkinsfile.public Outdated Show resolved Hide resolved
egonelbre
egonelbre previously approved these changes Sep 4, 2019
@egonelbre egonelbre changed the title work around git.apache.org failures jenkins: work around git.apache.org failures Sep 4, 2019
@egonelbre egonelbre dismissed stale reviews from zeebo and themself via 19c94ce September 4, 2019 07:03
@egonelbre egonelbre requested a review from zeebo September 4, 2019 08:12
@egonelbre egonelbre merged commit bf5a16b into master Sep 4, 2019
@egonelbre egonelbre deleted the thepaul/work-around-git.apache.org branch September 4, 2019 08:31
bryanchriswhite added a commit that referenced this pull request Sep 4, 2019
* storj/master:
  satellitedb: always release savepoint processing orders (#2936)
  Add TopperDEL to the CLA list (#2947)
  jenkins: work around git.apache.org failures (#2944)
  storagenode: add custom dial timeout for orders sending (#2939)
  docs/design: Adapt SN Downtime tracking to be a blueprint (#2931)
littleskunk pushed a commit that referenced this pull request Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed Request Code Review Code review requested Reviewer Can Merge If all checks have passed, non-owner can merge PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants