[GitHub] [hudi] lw309637554 commented on pull request #1810: [HUDI-875] Abstract hudi-sync-common, and support hudi-hive-sync
lw309637554 commented on pull request #1810: URL: https://github.com/apache/hudi/pull/1810#issuecomment-669248590 > @lw309637554 this seems ready? yes, but the test failed for some reason, i rerun it 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: us...@infra.apache.org
[GitHub] [hudi] lw309637554 commented on pull request #1810: [HUDI-875] Abstract hudi-sync-common, and support hudi-hive-sync
lw309637554 commented on pull request #1810: URL: https://github.com/apache/hudi/pull/1810#issuecomment-667553349 > @lw309637554 would you please rebase and fix the conflicts. okay, done 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: us...@infra.apache.org
[GitHub] [hudi] lw309637554 commented on pull request #1810: [HUDI-875] Abstract hudi-sync-common, and support hudi-hive-sync
lw309637554 commented on pull request #1810: URL: https://github.com/apache/hudi/pull/1810#issuecomment-663442107 > couple okay ,thanks 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: us...@infra.apache.org
[GitHub] [hudi] lw309637554 commented on pull request #1810: [HUDI-875] Abstract hudi-sync-common, and support hudi-hive-sync
lw309637554 commented on pull request #1810: URL: https://github.com/apache/hudi/pull/1810#issuecomment-661736470 @vinothchandar The pr is ready overall. Can you help to review ? 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: us...@infra.apache.org
[GitHub] [hudi] lw309637554 commented on pull request #1810: [HUDI-875] Abstract hudi-sync-common, and support hudi-hive-sync
lw309637554 commented on pull request #1810: URL: https://github.com/apache/hudi/pull/1810#issuecomment-659747098 > > this is a good point. I think remove hive dependency from hudi-spark and hudi-utilities is a another work . we can open another issue resolve it > > sounds good. follow up ticket: https://issues.apache.org/jira/browse/HUDI-1101 very nice 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: us...@infra.apache.org
[GitHub] [hudi] lw309637554 commented on pull request #1810: [HUDI-875] Abstract hudi-sync-common, and support hudi-hive-sync
lw309637554 commented on pull request #1810: URL: https://github.com/apache/hudi/pull/1810#issuecomment-659182664 > Hi @lw309637554 , I totally understand the importance of the backward capability. IMO, that will be great if we can remove the hive dependency from `hudi-spark` and `hudi-utilities`. If we treat `syncHive` separately, we still need to include some Hive related packages in these two modules. > I had this dependency issue before while I was testing the delta streamer. I didn't use Hive at all but I need to resolve some Hive dependency conflicts in my production environment. So I'd incline to sacrifice some backward capability and move all the Hive related packages to `hudi-hive-sync`. Do you think this is possible? > Happy to hear what you guys think. this is a good point. I think remove hive dependency from hudi-spark and hudi-utilities is a another work . we can open another issue resolve it 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: us...@infra.apache.org
[GitHub] [hudi] lw309637554 commented on pull request #1810: [HUDI-875] Abstract hudi-sync-common, and support hudi-hive-sync
lw309637554 commented on pull request #1810: URL: https://github.com/apache/hudi/pull/1810#issuecomment-658740024 @vinothchandar @garyli1019 I think in this PR the hudi-sync abstract is ready. Expect your review. Thanks 1. the module abstract is hudi-sync hudi-hive-sync/ hudi-dla-sync/ 2. about backwards compatible . a. about users code compatible: because do not modify the hivesync class name. Users do not need to modify these old code. b. about users pom dependency compatible: because do not modify the module name hudi-hive-sync. Users not need to modify pom dependency c. about users local jar files compatible: if users's local jars is do not shaded the hudi-hive-sync's Indirect dependence. Old users just need add the hudi-sync-common.jar to their directory. Just like hudi-utilities-bundle modiry in this PR. d. about sync parameters, just add new parameters such as --enable-sync as new default parameter,also backwards compatible --enable-hive-sync. 3. some others works ,such as update the doc. will do in other issues. what about your suggestion? 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: us...@infra.apache.org
[GitHub] [hudi] lw309637554 commented on pull request #1810: [HUDI-875] Abstract hudi-sync-common, and support hudi-hive-sync
lw309637554 commented on pull request #1810: URL: https://github.com/apache/hudi/pull/1810#issuecomment-656092089 @vinothchandar I meet some mistakes, just opened a new PR. In the new PR, I fix the build break. This is my thinks about backwards compatible . a. about users code compatible: because do not modify the hivesync class name. Users do not need to modify these old code. b. about users pom dependency compatible: because do not modify the module name hudi-hive-sync. Users not need to modify pom dependency c. about users local jar files compatible: if users's local jars is do not shaded the hudi-hive-sync's Indirect dependence. Old users just need add the hudi-sync-common.jar to their directory. Just like hudi-utilities-bundle modiry in this PR. Reply: we can do this to put the hudi-sync-common base class to hudi-common d. about sync parameters, just add new parameters. Compatible is ok. what about your suggestion? 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: us...@infra.apache.org
[GitHub] [hudi] lw309637554 commented on pull request #1810: [HUDI-875] Abstract hudi-sync-common, and support hudi-hive-sync
lw309637554 commented on pull request #1810: URL: https://github.com/apache/hudi/pull/1810#issuecomment-656086937 > @lw309637554 Thanks for your contribution! Overall looks good. > Open to discuss where to put these modules. I vote to put the base class to `hudi-common` and have separate modules for different query engines. thanks so much for your very valuable suggestion. 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: us...@infra.apache.org
