Github user arunmahadevan commented on the issue:
https://github.com/apache/storm/pull/1942
>We should check that it works with Flux. In other words, if Flux doesn't
support instantiating Class object and passing it, the change just breaks
backward compatibility.
Agree and pointed out in one of the earlier comments. One suggestion was to
have two methods,
1. setReader(Class<? extends FileReader> reader)
2. setReaderClass(String className)
>To be honest, accepting alias and class in one method doesn't look
intuitive. Users should memorize the alias (TEXT, SEQ) to use.
Right now the method accepts special strings "text", "seq" or class name,
which looks confusing and not type safe. The proposed patch addresses this by
accepting a FileReader class. I am not sure why cant we directly pass the
FileReader implementation instead of the class (like `setReader(FileReader
reader)`) and I believe flux will be able to handle this.
The current FileReader implementation is expected to have constructor with
a specific signature, which cannot be enforced at compile time. IMO, this
should also be fixed along with this patch by introducing some init method in
the FileReader interface.
Regarding backward compatibility, if we are fine to break it in the next
major version (2.0) we can keep this out of 1.x branch.
---
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.
---
