Github user jvwing commented on the pull request:
https://github.com/apache/nifi/pull/224#issuecomment-192777256
@mans2singh - Sorry I'm slow getting back to you. I have a few comments
about the most recent changes:
##### Tests
* The unit tests are much improved. Your mock implementation is better
than my suggested approach, and there has been a big improvement in code
coverage as a result.
* I think we could use one or two mocked tests for GetDynamoDB.
* Test coverage for unprocessed items is low, given that your design
targets the batch get/put/delete APIs and unprocessed items will be expected.
Earlier today, I thought that would be easily fixed. It turns out I don't have
solid knowledge of what unprocessed responses will really look like in
practice, and the AWS SDK is more confusing than helpful.
##### AbstractWriteDynamoDBProcessor
AbstractWriteDynamoDBProcessor has two methods - handleUnprocessedPutItems
and handleUnprocessedDeleteItems - and two subclasses, PutDynamoDB and
DeleteDynamoDB. I know that isn't how things started a couple weeks ago, it
used to be shared code, but isn't this a sign that we don't need an
AbstractWriteDynamoDBProcessor? Can we just move those methods to their
respective processors, or do you anticipate sharing them with additional
writing processors?
##### Relationships
What types of relationships have you considered besides "success" and
"failure"? A malformed input "failure" requires correction before retrying,
but unprocessed item failures might simply be retried. Also, GetDynamoDB might
have a separate relationship for "not found" as opposed to "failure". On the
good side, your inclusion of complete response codes in the output attributes
makes it possible to filter out the various failure modes. Additional
relationships could be added as features later.
I am still working on setting up some tests for unprocessed items.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
