juliuszsompolski commented on issue #25721: [WIP][SPARK-29018][SQL] Implement 
Spark Thrift Server with it's own code base on PROTOCOL_VERSION_V9
URL: https://github.com/apache/spark/pull/25721#issuecomment-544540930
 
 
   Looking into it a bit more, the PR description actually does describe most 
of the changes, but it's just hard to grasp through in one huge PR...
   
   Maybe separately have 4 PRs for:
   1. all the `gen/` code generated anew in `sql/hive-thriftserver/src/gen` - 
it has a new package now, so it will remain unused. (bonus: could we make these 
thrift files be generated on the fly? seems possible: 
https://stackoverflow.com/questions/18767986/how-can-i-compile-all-thrift-files-thrift-as-a-maven-phase)
   2. on top of that, all the Thriftserver code that is just translated from 
Java to Scala without changes - it has a new package now, so it will remain 
unused.
   3. on top of that, all the classes that are actually modified - and at this 
point stop compiling the v1.2.1 and v2.3.5 dirs and use the new code.
   4. At the end, on top of that delete the v1.2.1 and v2.3.5 dirs, and only 
then redirect the build to actually use the new code.
   
   For reviewing we could have 1, 2 and 3 separately on top of each other, plus 
a PR that does 1+2+3 that can be used for testing the new code. Mostly 3 will 
be what needs to be reviewed. Then 1, 2, 3 can be committed in quick succession 
(but I think it's worth keeping them in 3 separate commits).
   4 can be done afterwards for cleanup.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to