[GitHub] [nifi] takraj commented on pull request #6504: NIFI-10618: Add Asana connector
takraj commented on PR #6504: URL: https://github.com/apache/nifi/pull/6504#issuecomment-1353524643 > @takraj Thanks for the changes! Overall it looks good to me, except the last commit: > > > Fix constant IOException in onTrigger() when DistributedMapCache has no entries > > So there was an issue when `DistributedMapCache` was used (I tested it with Hazelcast cache and did not run into it). I debugged it a bit and the root cause seems to be that `GenericObjectSerDe.deserialize()` should handle empty byte arrays. Like in [CacheValueDeserializer](https://github.com/apache/nifi/blob/78be613a0f85b664695ea2cbfaf26163f9b8e454/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/FetchDistributedMapCache.java#L287-L296). Please fix it in the deserializer instead of cacthing the exception in `onTrigger()`. > > There are also other changes in the last commit whose purpose is unclear for me: `stateBackup` handling and moving the fetcher creation into `onTrigger()` instead of `onScheduled()`. If there is no good reason for them, I would suggest reverting the whole commit and adding the `GenericObjectSerDe` fix only. @turcsanyip Done. Please re-check. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] takraj commented on pull request #6504: NIFI-10618: Add Asana connector
takraj commented on PR #6504: URL: https://github.com/apache/nifi/pull/6504#issuecomment-1341645043 @turcsanyip I have attempted to address each of the discovered issues. Please check again. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] takraj commented on pull request #6504: NIFI-10618: Add Asana connector
takraj commented on PR #6504: URL: https://github.com/apache/nifi/pull/6504#issuecomment-1331352294 Rebased & squashed. Versions bumped to 1.20.0 -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] takraj commented on pull request #6504: NIFI-10618: Add Asana connector
takraj commented on PR #6504: URL: https://github.com/apache/nifi/pull/6504#issuecomment-1330964547 @exceptionfactory , @turcsanyip I have implemented support for `DistributedMapCacheClient` in 3ddb7bb. Please check my PR again, when you have time for 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] takraj commented on pull request #6504: NIFI-10618: Add Asana connector
takraj commented on PR #6504: URL: https://github.com/apache/nifi/pull/6504#issuecomment-1324788246 @exceptionfactory Thanks for the suggestion about `DistributedMapCacheClient`, I'll check soon if that approach could work. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] takraj commented on pull request #6504: NIFI-10618: Add Asana connector
takraj commented on PR #6504: URL: https://github.com/apache/nifi/pull/6504#issuecomment-1321801319 Thank you for reviewing @turcsanyip , > Also noted that the processor stores fingerprints for all seen Asana entries in the state (if I understand correctly). We try to avoid storing a large number of entries or big items in the state. I see the processor stores the fingerprints merged in a json in a single entry. What is the expected size of this state item? Yes, I believe you do see it correctly. There is no upper bound for the number of these items, and it scales with the number of items in Asana. The fingerprint of a single item is a fixed 64 bytes, encoded in Base64, which is about 37% larger than this, so 80-90 bytes per item is expected. The whole list is gzipped, but as the hashes are traditionally non-compressible, I wouldn't expect much size deflation in the case of a fingerprint list. (approx. 11-12k items would fit in 1 MB compressed state) Where Asana exposes the modification time (Tasks), or where the items are immutable (Attachments), I used the modification date / creation date as fingerprint, which are just text-represented integers with relatively small differences, and thus well compressible. In this case 4-8 bytes per item is expected. (approx. 130k items would fit in 1 MB compressed state) There is an alternative solution, where I could utilize the Asana event API, which only requires storing a relatively short ID (less than 100 bytes), and that would notify us when an item is added, updated, or removed. The drawback is that, in this case we would need to fetch the affected items one by one, which may cause rate-limitation problems if there are more than 1500 items to fetch per minute. Another drawback is that we could apply this mechanism only for items having a parent item. Teams, Users, Tags don't have such, therefore in those cases I still need to use the current method, with fingerprints. Switching to Event API requires a medium-sized change in the code. And almost all Asana integration tests would needed to be changed, because of mocks. What do you think? -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] takraj commented on pull request #6504: NIFI-10618: Add Asana connector
takraj commented on PR #6504: URL: https://github.com/apache/nifi/pull/6504#issuecomment-1279006794 @turcsanyip Thank you for reviewing. I have attempted to address all the issues you pointed out. Please check again. -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
