Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Use fork of Apache Thrift #2780

Merged
merged 1 commit into from
Feb 4, 2021
Merged

Use fork of Apache Thrift #2780

merged 1 commit into from
Feb 4, 2021

Conversation

jpkrohling
Copy link
Contributor

@jpkrohling jpkrohling commented Feb 3, 2021

Temporarily use a personal fork of Apache Thrift, tagged with 1.13.1, which includes the fix for THRIFT-5322 (apache/thrift#2292).
Fixes #2638 and fixes #2452.
The issue to track the reversion of this PR is #2781.

Signed-off-by: Juraci Paixão Kröhling juraci@kroehling.de

@jpkrohling jpkrohling requested a review from a team as a code owner February 3, 2021 10:22
@objectiser
Copy link
Contributor

@jpkrohling Would be good to create an issue to revert the change once an official version is available, and link within the description of this PR.

@codecov
Copy link

codecov bot commented Feb 3, 2021

Codecov Report

Merging #2780 (4c9096c) into master (8e5eb9b) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2780      +/-   ##
==========================================
+ Coverage   95.89%   95.90%   +0.01%     
==========================================
  Files         217      217              
  Lines        9625     9625              
==========================================
+ Hits         9230     9231       +1     
  Misses        326      326              
+ Partials       69       68       -1     
Impacted Files Coverage Δ
plugin/storage/integration/integration.go 77.34% <0.00%> (-0.56%) ⬇️
plugin/storage/badger/spanstore/reader.go 96.08% <0.00%> (+0.71%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e5eb9b...dc11c46. Read the comment docs.

@jpkrohling
Copy link
Contributor Author

@Mario-Hofstaetter, @zigmund, would you be open to testing an image with this PR, to confirm that your problems will get fixed?

@Mario-Hofstaetter
Copy link

Mario-Hofstaetter commented Feb 3, 2021

@jpkrohling Will do, hope to get it done by friday.
Is there a download available for a windows binary?

@yurishkuro
Copy link
Member

yurishkuro commented Feb 3, 2021

I thought we wanted the fork to be in github.com/jaegertracing. Why use personal fork?

@jpkrohling
Copy link
Contributor Author

I can push my fork to the jaegertracing organization, but it makes that somewhat too "official". I really want this to be temporary.

@yurishkuro
Copy link
Member

We can reflect that in the forked repo description. Btw pushing to a new repo from yours will not establish the forked link on GitHub.

@zigmund
Copy link

zigmund commented Feb 3, 2021

@jpkrohling yes, I can test image in few days.

@jpkrohling
Copy link
Contributor Author

Btw pushing to a new repo from yours will not establish the forked link on GitHub

Yes, sorry, I meant that I would fork Apache Thrift to jaegertracing org and push the tag there.

@jpkrohling
Copy link
Contributor Author

PR updated to use the fork from the jaegertracing organization.

Temporarily use a personal fork of Apache Thrift, tagged with 1.13.1, which includes the fix for THRIFT-5322 (apache/thrift#2292).
Fixes #2638 and #2452.

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
@yurishkuro yurishkuro merged commit c7c371d into jaegertracing:master Feb 4, 2021
@jpkrohling jpkrohling deleted the jpkrohling/use-fork-of-thrift branch July 28, 2021 19:21
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jaeger-agent reproducible memory leak in 1.21.0 Memory peaks on agent v1.19.2
5 participants