nifi icon indicating copy to clipboard operation
nifi copied to clipboard

Published 20 hours ago • verified publisher icon apache

NIFI-10234 implement PutIoTDB

Open xuanronaldo opened this issue 3 years ago • 8 comments

xuanronaldo avatar Sep 14 '22 02:09 xuanronaldo

I have updated the code to resolve conflicts. Please approve to run workflows again. @exceptionfactory

xuanronaldo avatar Sep 15 '22 02:09 xuanronaldo

Hi, I have pushed the code, and all checks have passed, and there are no conflicts with the base branch. What should I do next to merge the branch? @exceptionfactory

xuanronaldo avatar Sep 19 '22 02:09 xuanronaldo

Thanks for the updated work @xuanronaldo, I should be able to take a closer look at the latest version soon.

exceptionfactory avatar Sep 19 '22 12:09 exceptionfactory

I'm still planning on following up on this @xuanronaldo, but recent release efforts have taken precedence. Thanks for your patience.

exceptionfactory avatar Oct 03 '22 16:10 exceptionfactory

Sorry for replying you so late, because I got a lot of work to do. There is an excellent contributor @lizhizhou of our community would like to finish my job. Besides, he has commited pr to NiFi. So, I think you guys will work well together.

xuanronaldo avatar Oct 13 '22 02:10 xuanronaldo

Thanks for the reply, and for your work on this PR @xuanronaldo! When the final version is ready, we should be able to mention your contribution through a Co-authored-by line in the message.

Will additional work be done on this branch and PR, or will a new PR be submitted?

exceptionfactory avatar Oct 14 '22 16:10 exceptionfactory

All comments are resolved and new QueryIoTDB process is added.

lizhizhou avatar Oct 17 '22 05:10 lizhizhou

Thank you for addressing the comments @lizhizhou.

Can you remove the addition for the QueryIoTDB Processor? Although it also looks useful, it would be better to resolve and merge the PutIoTDB Processor first before introducing another processor, which will require additional evaluation. Once the initial PR is complete, then introducing the Query Processor in a new PR would help focus on that capability.

exceptionfactory avatar Oct 18 '22 13:10 exceptionfactory

Remove the QueryIoTDB

lizhizhou avatar Oct 20 '22 07:10 lizhizhou

@exceptionfactory Is there any I should do before merging the PR?

lizhizhou avatar Oct 22 '22 13:10 lizhizhou

Thanks for the recent updates @lizhizhou, I will take a closer look soon. At the moment, there is a merge conflict with nifi-nar-bundles/pom.xml, can you take a look and resolve?

exceptionfactory avatar Oct 24 '22 13:10 exceptionfactory

Thanks for the recent updates @lizhizhou, I will take a closer look soon. At the moment, there is a merge conflict with nifi-nar-bundles/pom.xml, can you take a look and resolve?

Resolved. Thanks.

lizhizhou avatar Oct 25 '22 02:10 lizhizhou

@exceptionfactory Everything is clean now.

lizhizhou avatar Oct 27 '22 08:10 lizhizhou

@exceptionfactory All check is clean. Can you help to merge the code?

lizhizhou avatar Nov 16 '22 02:11 lizhizhou

Thanks for the updates @lizhizhou, the record handling approach appears to be improved, I will take a closer look at this soon.

exceptionfactory avatar Nov 16 '22 22:11 exceptionfactory

@exceptionfactory Thanks for the review and all comments are resolved.

lizhizhou avatar Dec 21 '22 02:12 lizhizhou

@lizhizhou I pushed a minor update to correct build errors related to removed CompressionType values, and also adjusted the unit test class name to match standard conventions. I will continue reviewing other changes.

exceptionfactory avatar Dec 30 '22 22:12 exceptionfactory

It looks like the current build is failing due to differing parent POM versions. Recommend rebasing against the current main branch, which would be a good time to squash commits and remove merge commits from the history.

exceptionfactory avatar Dec 30 '22 22:12 exceptionfactory

@lizhizhou It looks like the latest updates included many historical commits, please rebase and force-push to correct the branch history.

exceptionfactory avatar Jan 02 '23 18:01 exceptionfactory

Thanks for adding the Time Field property @lizhizhou.

To simplify the final stages of the review, can you squash all changes into a single commit? The current list of 27 commits still includes references to other pull requests, so this is a good opportunity to squash all changes as this nears completion.

exceptionfactory avatar Jan 04 '23 18:01 exceptionfactory

Thanks for adding the Time Field property @lizhizhou.

To simplify the final stages of the review, can you squash all changes into a single commit? The current list of 27 commits still includes references to other pull requests, so this is a good opportunity to squash all changes as this nears completion.

All changes have been merge to single commit now.

lizhizhou avatar Jan 06 '23 06:01 lizhizhou

@exceptionfactory I add the Apache IoTDB Query in https://github.com/apache/nifi/pull/6844. Please help to review the code. Thanks a lot.

lizhizhou avatar Jan 13 '23 01:01 lizhizhou