[SPARK-18652][PYTHON] Include the example data and third-party licenses in pyspark package. #16082

lins05 · 2016-11-30T17:16:05Z

What changes were proposed in this pull request?

Since we already include the python examples in the pyspark package, we should include the example data with it as well.

We should also include the third-party licences since we distribute their jars with the pyspark package.

How was this patch tested?

Manually tested with python2.7 and python3.4

$ ./build/mvn -DskipTests -Phive -Phive-thriftserver -Pyarn -Pmesos clean package
$ cd python
$ python setup.py sdist
$ pip install  dist/pyspark-2.1.0.dev0.tar.gz 

$ ls -1 /usr/local/lib/python2.7/dist-packages/pyspark/data/
graphx
mllib
streaming

$ du -sh /usr/local/lib/python2.7/dist-packages/pyspark/data/
600K    /usr/local/lib/python2.7/dist-packages/pyspark/data/

$ ls -1  /usr/local/lib/python2.7/dist-packages/pyspark/licenses/|head -5                                  
LICENSE-AnchorJS.txt
LICENSE-DPark.txt
LICENSE-Mockito.txt
LICENSE-SnapTree.txt
LICENSE-antlr.txt

SparkQA · 2016-11-30T17:51:45Z

Test build #69412 has finished for PR 16082 at commit 43019db.

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

SparkQA · 2016-11-30T18:12:13Z

Test build #69415 has finished for PR 16082 at commit 9735e20.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-11-30T18:57:31Z

Test build #69420 has finished for PR 16082 at commit 90ca61d.

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

SparkQA · 2016-11-30T19:49:16Z

Test build #69416 has finished for PR 16082 at commit ab51ae3.

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

lins05 · 2016-12-02T15:23:23Z

@holdenk @rxin Can we merge this?

holdenk

Thanks for taking this on. I've got a few minor questions but overall looks useful.

holdenk · 2016-12-02T16:05:39Z

python/MANIFEST.in

@@ -17,6 +17,8 @@
 global-exclude *.py[cod] __pycache__ .DS_Store
 recursive-include deps/jars *.jar
 graft deps/bin
+graft deps/data
+graft deps/licenses


So we later limit this to text files, maybe recursive-include would be better?

holdenk · 2016-12-02T16:07:27Z

python/setup.py


+data_files = glob.glob(os.path.join(LICENSES_PATH, "*"))


Is this necessary? None of the other packages we're making seem to need it.

I think the question is whether whatever product this produces includes copies of third-party software, as in an assembly jar? if so the licenses should be included. If no Java code is being packaged, most of this won't be relevant.

Oh no I mean, including the license files is reasonable its more is it necessary to use data_files here (and down bellow) when all of the other packages we are producing simply use package_data to specify the backing files.

Makes sense, I'll remove the data_files related to make them consistent.

holdenk · 2016-12-02T16:09:36Z

python/MANIFEST.in

@@ -17,6 +17,8 @@
 global-exclude *.py[cod] __pycache__ .DS_Store
 recursive-include deps/jars *.jar
 graft deps/bin
+graft deps/data


Since we limit this to *.data and *.txt later would it make sense to restrict this here as well? That being said I can see an argument that we might have csv or tsv files in the future in which case we'd want to switch the include downstream to be * rather than just txt / data.

Yeah, I think limiting it in one place would be enough.

On a second thought I think it would be better to keep all the packages consistent (specifying the patterns to include in both MANIFEST.in and setup.py), so I changed the graft to recursive-include with proper patters as you suggested.

holdenk · 2016-12-02T16:09:55Z

python/setup.py

            'pyspark.examples.src.main.python': ['*.py', '*/*.py']},
+        data_files=[('', data_files)],


As mentioned above - do we really need this directive?

holdenk · 2016-12-02T17:04:26Z

Also just if you can tag your Python PRs with [PYTHON] it makes searching for them a bit faster.

holdenk · 2016-12-04T10:42:48Z

python/setup.py

 SCRIPTS_TARGET = os.path.join(TEMP_PATH, "bin")
 JARS_TARGET = os.path.join(TEMP_PATH, "jars")
 EXAMPLES_TARGET = os.path.join(TEMP_PATH, "examples")
-
+DATA_TARGET = os.path.join(TEMP_PATH, "data")
+LICENSES_PATH = os.path.join(SPARK_HOME, "licenses")


Super minor, and sorry I didn't notice it earlier, but we define all of the _PATHs before the target - just for someone skimming the code would be good to keep the current flow.

Done. Good catch!

holdenk · 2016-12-04T10:50:03Z

One minor nit, but other than that LGTM - thank you for taking the time to do this @lins05 :)

holdenk · 2016-12-04T11:02:40Z

This looks good to me, maybe @srowen, @davies, or @JoshRosen could do a final pass/merge?

SparkQA · 2016-12-04T11:18:51Z

Test build #69636 has finished for PR 16082 at commit 5706d8c.

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

SparkQA · 2016-12-04T11:34:57Z

Test build #69637 has finished for PR 16082 at commit 8d3ef53.

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

…es in pyspark package. ## What changes were proposed in this pull request? Since we already include the python examples in the pyspark package, we should include the example data with it as well. We should also include the third-party licences since we distribute their jars with the pyspark package. ## How was this patch tested? Manually tested with python2.7 and python3.4 ```sh $ ./build/mvn -DskipTests -Phive -Phive-thriftserver -Pyarn -Pmesos clean package $ cd python $ python setup.py sdist $ pip install dist/pyspark-2.1.0.dev0.tar.gz $ ls -1 /usr/local/lib/python2.7/dist-packages/pyspark/data/ graphx mllib streaming $ du -sh /usr/local/lib/python2.7/dist-packages/pyspark/data/ 600K /usr/local/lib/python2.7/dist-packages/pyspark/data/ $ ls -1 /usr/local/lib/python2.7/dist-packages/pyspark/licenses/|head -5 LICENSE-AnchorJS.txt LICENSE-DPark.txt LICENSE-Mockito.txt LICENSE-SnapTree.txt LICENSE-antlr.txt ``` Author: Shuai Lin <[email protected]> Closes #16082 from lins05/include-data-in-pyspark-dist. (cherry picked from commit bd9a4a5) Signed-off-by: Sean Owen <[email protected]>

srowen · 2016-12-06T22:10:32Z

Merged to master, and 2.1 as it's relevant to packaging of the release

…es in pyspark package. ## What changes were proposed in this pull request? Since we already include the python examples in the pyspark package, we should include the example data with it as well. We should also include the third-party licences since we distribute their jars with the pyspark package. ## How was this patch tested? Manually tested with python2.7 and python3.4 ```sh $ ./build/mvn -DskipTests -Phive -Phive-thriftserver -Pyarn -Pmesos clean package $ cd python $ python setup.py sdist $ pip install dist/pyspark-2.1.0.dev0.tar.gz $ ls -1 /usr/local/lib/python2.7/dist-packages/pyspark/data/ graphx mllib streaming $ du -sh /usr/local/lib/python2.7/dist-packages/pyspark/data/ 600K /usr/local/lib/python2.7/dist-packages/pyspark/data/ $ ls -1 /usr/local/lib/python2.7/dist-packages/pyspark/licenses/|head -5 LICENSE-AnchorJS.txt LICENSE-DPark.txt LICENSE-Mockito.txt LICENSE-SnapTree.txt LICENSE-antlr.txt ``` Author: Shuai Lin <[email protected]> Closes apache#16082 from lins05/include-data-in-pyspark-dist.

lins05 added 2 commits December 1, 2016 01:12

[SPARK-18652] Include the data in pyspark package.

43019db

Also include the third-party licenses in pyspark package.

9735e20

lins05 changed the title ~~[SPARK-18652] Include the data in pyspark package.~~ [SPARK-18652] Include the example data and third-party licenses in pyspark package. Nov 30, 2016

lins05 force-pushed the include-data-in-pyspark-dist branch from dc7127c to 7c2d0fc Compare November 30, 2016 17:44

Use graft instead of include

ab51ae3

lins05 force-pushed the include-data-in-pyspark-dist branch from 7c2d0fc to ab51ae3 Compare November 30, 2016 17:45

Fixed the error when using python3.

90ca61d

srowen approved these changes Dec 1, 2016

View reviewed changes

holdenk requested changes Dec 2, 2016

View reviewed changes

lins05 changed the title ~~[SPARK-18652] Include the example data and third-party licenses in pyspark package.~~ [SPARK-18652][PYTHON] Include the example data and third-party licenses in pyspark package. Dec 4, 2016

Address review comments.

5706d8c

holdenk reviewed Dec 4, 2016

View reviewed changes

CR

8d3ef53

srowen approved these changes Dec 6, 2016

View reviewed changes

asfgit closed this in bd9a4a5 Dec 6, 2016

lins05 deleted the include-data-in-pyspark-dist branch December 8, 2016 15:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-18652][PYTHON] Include the example data and third-party licenses in pyspark package. #16082

[SPARK-18652][PYTHON] Include the example data and third-party licenses in pyspark package. #16082

lins05 commented Nov 30, 2016 •

edited

Loading

SparkQA commented Nov 30, 2016

SparkQA commented Nov 30, 2016

SparkQA commented Nov 30, 2016

SparkQA commented Nov 30, 2016

lins05 commented Dec 2, 2016

holdenk left a comment

holdenk Dec 2, 2016

lins05 Dec 4, 2016

holdenk Dec 2, 2016

srowen Dec 2, 2016

holdenk Dec 2, 2016

lins05 Dec 4, 2016

holdenk Dec 2, 2016

lins05 Dec 4, 2016

lins05 Dec 4, 2016

holdenk Dec 2, 2016

holdenk commented Dec 2, 2016

holdenk Dec 4, 2016

lins05 Dec 4, 2016

holdenk commented Dec 4, 2016

holdenk commented Dec 4, 2016

SparkQA commented Dec 4, 2016

SparkQA commented Dec 4, 2016

srowen commented Dec 6, 2016

		'pyspark.examples.src.main.python': ['.py', '/*.py']},
		data_files=[('', data_files)],

[SPARK-18652][PYTHON] Include the example data and third-party licenses in pyspark package. #16082

[SPARK-18652][PYTHON] Include the example data and third-party licenses in pyspark package. #16082

Conversation

lins05 commented Nov 30, 2016 • edited Loading

What changes were proposed in this pull request?

How was this patch tested?

SparkQA commented Nov 30, 2016

SparkQA commented Nov 30, 2016

SparkQA commented Nov 30, 2016

SparkQA commented Nov 30, 2016

lins05 commented Dec 2, 2016

holdenk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holdenk commented Dec 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holdenk commented Dec 4, 2016

holdenk commented Dec 4, 2016

SparkQA commented Dec 4, 2016

SparkQA commented Dec 4, 2016

srowen commented Dec 6, 2016

lins05 commented Nov 30, 2016 •

edited

Loading