Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for IBatchSpout to redstorm #57

Closed
wants to merge 10 commits into from

Conversation

dinedal
Copy link
Contributor

@dinedal dinedal commented Dec 6, 2012

This is required for basic non-transactional trident spouts defined in jruby.

@colinsurprenant
Copy link
Owner

thanks @dinedal for the contribution! looks straightforward, I'll review it shortly and merge it.

Colin

@dinedal
Copy link
Contributor Author

dinedal commented Dec 10, 2012

@colinsurprenant Please also review the linear DRPC support I've added.

@colinsurprenant
Copy link
Owner

@dinedal the thing is, Storm 0.8.1 deprecated the LinearDRPC and transactional topologies. We have to see what makes sense now and moving forward. Also @adsummos added lors of stuff to support transactional topologies see adsummos@f2bd64a - I haven't had time to review all this yet in details.

@dinedal
Copy link
Contributor Author

dinedal commented Dec 11, 2012

@colinsurprenant Saw his work, looks like he's covered the transactional stuff, but missed the DRPC stuff.

I understand that LinearDRPC is to be deprecated, and it's in my roadmap to work on the Trident classes for Redstorm as well. If you want to comment on my code style / other things to make sure the Trident work I do gets accepted to the main branch easier, please do.

The (I)BatchSpout implementation is going to be used as part of Trident stuff though, to be sure, since it's the non-transactional building block there. Opaque and Transactional spouts need to be done, but BatchSpout works in redstorm native mode, there's just no Trident DSL yet.

@adsummos
Copy link
Contributor

I have a use now for the Trident stuff, have you started to work on it yet? If you have, do you have a feature branch you can push out? If not, I'm going to start working on it.

@colinsurprenant
Copy link
Owner

no, haven't look into Trident yet. We should move Trident discussions into issue #46.

@kb
Copy link

kb commented Apr 10, 2013

@colinsurprenant Curious if there's a plan in place to merge this work?

cc/ @dinedal

@colinsurprenant
Copy link
Owner

I so badly need to catch up on this.

«please wait, your call is important to us» :-P

But seriously, I will reserve time either this weekend or early next week for RedStorm...

@colinsurprenant
Copy link
Owner

Just to let you know I am now catching up on redstorm.

@ghost ghost assigned colinsurprenant May 7, 2013
@colinsurprenant
Copy link
Owner

merged your changes locally, working on it.

@colinsurprenant
Copy link
Owner

this is currently merged into the v0.6.5 branch. I did not include your generator code for now. it's an interesting idea but will not test it for 0.6.5. Also, batch bolt support was already added from PR #52.

Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants