storm icon indicating copy to clipboard operation
storm copied to clipboard

Update Event Hubs spout and bolt to use modern Event Hubs client

Open JamesBirdsall opened this issue 5 years ago • 3 comments

We have customers using Storm 1.1.x, but the Event Hubs bolt and spout they were using was built from someone's private repo, where they had rebuilt the bolt and spout to use the current Event Hubs client (artifact azure-eventhubs) instead of the old one (artifact eventhubs-client).

This PR is intended to recover that codebase into the public Storm repo, and update it to the current version of azure-eventhubs.

JamesBirdsall avatar May 01 '19 00:05 JamesBirdsall

Hi, thanks for the patch. Upgrading the eventhubs client sounds good, but we need this to go in the newer branches first. The fix needs to go in master, and 1.x-branch, then we can decide whether we want it in 1.1.x as well. You can leave this PR open if you like, but we need separate PRs targeting those branches.

Please also raise an issue at https://issues.apache.org/jira for this, and then rename the PR and commit messages to include the issue number. Ideally also squash to one commit, as that makes it easier for us to cherry pick fixes.

srdo avatar May 01 '19 05:05 srdo

Thanks for the feedback! One question: master is Storm 2.x -- are bolts/spouts sufficiently compatible between 1.x and 2.x that a change in 2.x can be pulled back into 1.x? Or would two separate fixes/PRs be required?

JamesBirdsall avatar May 02 '19 17:05 JamesBirdsall

I would expect any API changes to be very minor. As far as I recall, the only API change you're likely to see as an implementer of a spout is that the open method's Map conf is now Map<String, Object> conf.

That said, 1.x is targeting Java 7, while master is targeting Java 8. If you don't use any Java 8 features, a backport should be straightforward.

Most likely, you can just take the changes you're making here and target them at the master branch. Two PRs are probably best though.

srdo avatar May 02 '19 18:05 srdo

Given the low amount of resources to work on Storm, I am going to close this PR against 1.x

rzo1 avatar Oct 23 '23 14:10 rzo1