zeppelin
zeppelin copied to clipboard
[ZEPPELIN-6060] Integrate Thrift code generation with Maven build and enhance genthrift.sh
What is this PR for?
This PR aims to enhance the build process in Zeppelin by integrating Thrift Java file generation into the Maven build workflow. It also updates the genthrift.sh script to automatically handle Thrift installation. These changes are intended to reduce manual errors and ensure consistent build environments.
What type of PR is it?
Improvement
Todos
- [x] - Add
exec-maven-plugintopom.xmlto automate the generation of Java files from Thrift IDL during the Maven build process. - [x] - Enhance
genthrift.shto support automatic Thrift installation, including improved checks for Thrift being installed.
What is the Jira issue?
- https://issues.apache.org/jira/projects/ZEPPELIN/issues/ZEPPELIN-6060
How should this be tested?
- Verify Thrift installation
- Confirm Thrift .java file generation through Maven
Questions:
- Does the license files need to update? NO
- Is there breaking changes for older versions? NO
- Does this needs documentation? NO
Installation of packages via yum or apt-get requires root rights.
Maven runs without root rights.
I also looked at the problem with thrift some time ago and hoped that the Java classes could be generated with a Maven plugin. Similar to GRPC. Unfortunately, I have not found anything.
My goal was just to have the thrift files in git and generate the thrift classes each time. But due to the installation process I don't see a way to do this. How do other Thrift projects implement it?
Did you come up with genthrift.sh script all by yourself, or is there reference(or similar script) you referred to and share with us? Like @Reamer says, it would be good to refer to how other projects handle situations like this.
integrating thrift codegen into the regular build process makes the developer hard to set up the environment, because thrift community does not publish the convenient thrift binary on releases, the downstream developers have to build thrift from source or get from the platform package managers like apt/brew, neither ways are convenient due to additional native deps requirements or only a few of versions providing.
given the generated thrift code won't change frequently, tracking the generated thrift code and providing a script to regenerate it is a common way. I think we can make some improvements in genthrift.sh, for example, make it working dir agnostic, allow passing env THRIFT_EXEC to use a custom thrift
Thank you for the discussion. My first goal is to ask him to divide thrift definition and generated files with zeppelin-intepreter module to minimize itself and clean up the module. During this job, I thought it would be better to generate thrift files easier. By the way, in my past experience, I provided all binaries for thrift compilers into a repository. Can we choose a similar way? You also can use docker to build the thrift files. Current implementation looks good personally but as others said it might be a quite difficult. WDYT?
@jongyoul Installing thrift and helping with its setup could be quite complicated and challenging. Therefore, I considered using Docker, but the current Docker thrift image has been deprecated since version 0.12.0, and there is no support for the current version, 0.13.0.
However, through @Reamer's review, I found an alternative approach by exploring the thrift-related code here: https://github.com/apache/hbase/blob/master/hbase-thrift/pom.xml
Looking at this code, instead of supporting the installation directly, we could consider an approach where we simply check if thrift is installed and proceed only if it is. This method could reduce the build burden for users who don't have thrift installed while maintaining build consistency for those who do. However, this approach might only be feasible because, in the example above, the thrift code is separated into its own module.
This pull request has been inactive for over a year. If no further activity occurs within the next 30 days, it will be automatically closed. If you believe this PR is still relevant, please feel free to leave a comment or make an update. Thank you!
This pull request has been automatically closed due to prolonged inactivity (over one year without updates). If you feel this was done in error or would like to continue the discussion, feel free to reopen it. Thank you for your contributions!