nsq_to_nsq: multiple topics support #945
Conversation
|
Tested locally with a few nsqd, topics, and made-up messages, both with and without |
|
LGTM, thanks |
|
FYI, GitHub added a feature where maintainers can push to PR branches, see https://github.com/blog/2247-improving-collaboration-with-forks. I've been using this to cleanup commits for recently merged PRs from contributors and I think it would have worked for you on #912 @ploxiln. |
|
Ah, I didn't think of that, will try that next time |
Multiple --topic flags can be passed. If --destination-topic is specified, all consumed topics will be published to that single destination-topic name at the destination. If --destination-topic is *not* specified, all consumed topics will be published to the corresponding topic name topic at the destination.
and rename "publisherHandlerRef" to "publisher"
ploxiln
force-pushed
the
nsq_to_nsq_multi
branch
from
575b3d8
to
e7434ef
Compare
|
@ploxiln interested in applying the same multi-topic changes to |
|
Not anytime soon, I just want to finish what I've started (I haven't forgotten the "contrib modules" thing :)
|
|
@ploxiln Thanks for cleaning up the diff, I definitely forgot to change that back. I will make sure I remember not to increase the diff next time. @mreiferson @ploxiln Can I take a stab on supporting multi topic for nsq_to_http ? |
|
@jlr52 go for it! |
my proposed changes to #912
I squashed #912 down to one commit, then added my proposed changes, mostly code-style. See my commit messages. If we agree on these changes, I'll squash most of my changes (all but the last commit) into @jlr52's commit (which will keep his authorship).
The biggest change is to re-inline
commandLineValidation()andinitConsumerAndHandler()intomain()- this is admittedly subjective, but the reasoning is that it reduces the diff against master to about half of what #912 was.I'll do my own quick manual tests now (but I'm confident :). Take a quick look and let me know what you think @jlr52 @mreiferson