Skip to content

Conversation

@srowen
Copy link
Member

@srowen srowen commented Oct 2, 2017

What changes were proposed in this pull request?

Move flume behind a profile, take 2. See #19365 for most of the back-story.

This change should fix the problem by removing the examples module dependency and moving Flume examples to the module itself. It also adds deprecation messages, per a discussion on dev@ about deprecating for 2.3.0.

How was this patch tested?

Existing tests, which still enable flume integration.

@SparkQA
Copy link

SparkQA commented Oct 2, 2017

Test build #82392 has finished for PR 19412 at commit 4b45f9c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member Author

srowen commented Oct 3, 2017

@vanzin might want a look on this second try, or @squito

@vanzin
Copy link
Contributor

vanzin commented Oct 4, 2017

I'm not really convinced that adding this profile is really achieving much. After all, the flume connector doesn't end up in the Spark packaging, only in maven central, and the new profile seems to be enabled in all the scripts anyway. So nothing seems to be changing much aside from deprecating the connector.

That being said, I don't really care one way or another as long as the code is there. Might want to test with maven too.

@srowen
Copy link
Member Author

srowen commented Oct 5, 2017

I think the argument for it if anything is that it's a) deprecated, so should kinda be optional to build, and b) this would simply be consistent with how other external/* modules are handled. For Spark 2.x yes there isn't and shouldn't be an actual change to the outputs.

There's a legitimate separate question here about whether it should be deprecated? my sense is yes, to leave the option to remove it in Spark 3.0, which would probably follow 2.3. I recall something about flume-ng uses an old version of Netty and it's the thing blocking updating it for all of Spark, but I may be misremembering the detail there.

Yeah this passed a Maven test build as well as dev/run-tests now.

@srowen
Copy link
Member Author

srowen commented Oct 6, 2017

Merged to master

@asfgit asfgit closed this in 0c03297 Oct 6, 2017
@srowen srowen deleted the SPARK-22142.2 branch October 7, 2017 18:03
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