Github user Parth-Brahmbhatt commented on the pull request:
https://github.com/apache/storm/pull/365#issuecomment-70752983
@HeartSaVioR and @dashengju Thanks for the patch and I think overall the
code looks good.
I am little confused about the core bolt. You have a trident state and
query function but in the core implementation you just provide an abstract
bolt. Any reason why the core functionality can not be extended to support the
basic persist and lookup features out of the box? You already have a mapper
interface for trident and instead of taking the tridentuple as input you could
change the type to ITuple so it will be able to support both trident states and
core bolt. It is ok to leave this implementation as is and file a jira to add
the core persist and lookup bolts later or document why it is not a good idea
to do so.
Also I think the README should be more detailed. you probably want to
document that users trying to use core features needs to extend
AbstractRedisBolt and how they should be using methods from the parent class.
The trident section should cover the query functions and your probably want to
remove ShardedRedis related classes from README as they don't exist.
---
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.
---
