Skip to content

Conversation

@srdo
Copy link
Contributor

@srdo srdo commented Jul 19, 2017

This is the 1.x subtask of https://issues.apache.org/jira/browse/STORM-2542

The first commit is from #2223. I'd like us to switch to using manual partitioning in 1.2.0 as the default rather than waiting for 2.0, since the subscribe API seems to cause people issues. It's not something I feel strongly about though, so if it's too big of a change for a minor version release, let me know and I'll revert it.

@srdo srdo force-pushed the STORM-2542-1.x branch from 53d5a65 to 273383e Compare July 19, 2017 10:38
@srdo srdo changed the title STORM-2542: Deprecate KafkaConsumer.subscribe API option, make KafkaConsumer.assign the default STORM-2640: Deprecate KafkaConsumer.subscribe API option, make KafkaConsumer.assign the default Jul 19, 2017
@srdo srdo force-pushed the STORM-2542-1.x branch from 273383e to 4b006f7 Compare July 19, 2017 18:39
@hmcl
Copy link
Contributor

hmcl commented Jul 20, 2017

@srdo since the dynamic partition assignment does not work anyways, and since Storm 2.0 is a major release, I wonder if we shouldn't simply remove these classes. It is not backwards compatible, but if it doesn't really work, what's the harm? I am not a big fan of, from very early releases, leaving deprecated code around, which just becomes a source of confusion and harder to maintain. At the very least we should plan a future release to remove it for good.

@hmcl
Copy link
Contributor

hmcl commented Jul 20, 2017

The commit with title "Cherry pick STORM-2542" should have the same title as in master. We should also incorporate the Java 7 compatibility changes in the commit itself. Otherwise it's really hard to track which commits really belong to the JIRA.

@srdo
Copy link
Contributor Author

srdo commented Jul 20, 2017

@hmcl The deprecated classes are removed in #2151, this is a 1.x PR to deprecate them before they get removed.

I thought the PR might be easier to review with some separate commits. I'll squash down to one commit with the PR's title once it has been reviewed.

@hmcl
Copy link
Contributor

hmcl commented Jul 25, 2017

+1

@srdo srdo force-pushed the STORM-2542-1.x branch from 4b006f7 to 2ebf226 Compare July 25, 2017 08:40
@asfgit asfgit merged commit 2ebf226 into apache:1.x-branch Jul 26, 2017
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.

3 participants