-
Notifications
You must be signed in to change notification settings - Fork 102
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
Update Thrift to version 0.16 #657
Update Thrift to version 0.16 #657
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GitHub CI should also be updated to use Thrift 16.
empty lines
@mcserep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now, nice work!
Update Thrift to version 0.16.
Since version 0.14, Thrift does not generate certain files, causing an error in CMake. To solve this, I made a wrapper function around the generation logic, so if some of the specified files are not generated, we manually create them.
There was an undocumented breaking change involving the java transport protocol implementations. The indexer service didn't handle all the exceptions correctly, leading to an API mismatch.
Since version 0.15, the javascript library provided by the
thrift
npm package also ships with the nodejs bindings. For environment detection, it internally uses a package calledbrowser-or-node
. However, this plugin needs to be bundled with the generated javascript code, making it impossible to directly use it in the old webgui by simply referencing the thrift.js library downloaded from npm. Such change requires the introduction of a build system (likewebpack
orbrowserify
), and the proper conversion of CommonJS modules to AMD, so it goes beyond the scope of this PR. However, a traditional browser version of the library can still be acquired by downloading it directly from the Thrift repository with the proper version. I've decided to simply grab it from here and put it into the appropriatescripts
directory.The new webgui doesn't produce this behavior, as expected.
fixes #630