Skip to content

[MINOR] Removed unused profiles from spark/pom.xml - #1301

Closed
jongyoul wants to merge 3 commits into
apache:masterfrom
jongyoul:minor/remove-unused-profile-spark-pom
Closed

[MINOR] Removed unused profiles from spark/pom.xml#1301
jongyoul wants to merge 3 commits into
apache:masterfrom
jongyoul:minor/remove-unused-profile-spark-pom

Conversation

@jongyoul

Copy link
Copy Markdown
Member

What is this PR for?

Making spark/pom.xml simple

What type of PR is it?

[Refactoring]

Todos

  • - Simplified pom.xml in spark

What is the Jira issue?

N/A

How should this be tested?

No test. CI should be green

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@jongyouljongyoul closed this Aug 8, 2016
@jongyouljongyoul reopened this Aug 8, 2016
@jongyoul

Copy link
Copy Markdown
MemberAuthor

@jongyouljongyoul closed this Aug 8, 2016
@jongyouljongyoul reopened this Aug 8, 2016
@jongyoul

Copy link
Copy Markdown
MemberAuthor

@bzz

bzz commented Aug 10, 2016

Copy link
Copy Markdown
Member

Simplification looks great to me!

But one thing is not clear - i.e a MapR profiles are not useful any more?

IDK but my guess is that they were added there for the reason (compatibility \w MapR build of Hadoop distro) - we should remove those at least with explanation of the reason, why that is done and how it affects people \w those distro, how do you think?

Looks like candidate for flaky-test labeled JIRA issue in zeppelin-server:

Failed tests: ZeppelinSparkClusterTest.zRunTest:204 expected:<FINISHED> but was:<ERROR>
Tests run: 65, Failures: 1, Errors: 0, Skipped: 0

@jongyoul

Copy link
Copy Markdown
MemberAuthor

@bzz Basically, Spark module doesn't have to need hadoop version because we only use Spark repl, core and so on not including specific hadoop version. spark-dependencies only needs specific hadoop version. Thus, specifying hadoop version is not necessary for it.

@bzz

bzz commented Aug 11, 2016

Copy link
Copy Markdown
Member

Thank you for explanation! Makes sense.

Looks great to me.

May be it's worth to git rebase master as CI seems to be recently fixed.

@zjffdu

Copy link
Copy Markdown
Contributor

Definitely should remove profile yarn. I got classpath issue when enabling yarn profile.

And we need to update the docs accordingly.

@bzz

bzz commented Aug 16, 2016

Copy link
Copy Markdown
Member

@jongyoul do you want to take of docs update that @zjffdu mention in this PR as well?

@jongyoul

Copy link
Copy Markdown
MemberAuthor

@zjffdu@bzz I've already removed yarn profile. Have I missed something?

@jongyouljongyoul reopened this Aug 19, 2016
@jongyoul

Copy link
Copy Markdown
MemberAuthor

And about docs, it never change some behaviour. I think another issue linked above is a better point to update docs.

Comment threadspark/pom.xml

<!-- include sparkr in the build -->
<profile>
<id>sparkr</id>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is sparkr needed here ? Because I notice there's another sparkr profile in spark-dependencies/pom.xml

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because of selecting interpreter-setting.json if/if not we use sparkR and this is because we have two different SparkR versions.

@felixcheung

Copy link
Copy Markdown
Member

I thought we do need cassandra-spark-*@doanduyhai ?

@jongyoul

Copy link
Copy Markdown
MemberAuthor

@felixcheung What I want to remove is duplicated from spark-dependencies/pom.xml. This deletion never hurt current features. It just removes duplicated codes.

@jongyoul

Copy link
Copy Markdown
MemberAuthor

Merging if there's no more discussion.

@bzz

bzz commented Aug 26, 2016

Copy link
Copy Markdown
Member

Quick question: just to double-check - this should not affect users following published build instructions like https://www.mapr.com/blog/building-apache-zeppelin-mapr-using-spark-under-yarn , right?

In a way like in ZEPPELIN-1353

@jongyoul

Copy link
Copy Markdown
MemberAuthor

@bzz Yes, what you mentioned is related to #1339 which will make a change to build custom Spark.

@jongyoul
jongyoulforce-pushed the minor/remove-unused-profile-spark-pom branch from 8de6cf8 to e55e307CompareAugust 29, 2016 09:03
@jongyoul
jongyoulforce-pushed the minor/remove-unused-profile-spark-pom branch from e55e307 to 10c7bb7CompareAugust 31, 2016 08:32
Sign up for freeto 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

@jongyoul@bzz@zjffdu@felixcheung