Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lins05 wants to merge 6 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
43019db
[SPARK-18652] Include the data in pyspark package.
lins05 Nov 30, 2016
9735e20
Also include the third-party licenses in pyspark package.
lins05 Nov 30, 2016
ab51ae3
Use graft instead of include
lins05 Nov 30, 2016
90ca61d
Fixed the error when using python3.
lins05 Nov 30, 2016
5706d8c
Address review comments.
lins05 Dec 4, 2016
8d3ef53
CR
lins05 Dec 4, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Also include the third-party licenses in pyspark package.
  • Loading branch information
@lins05
lins05 committed Nov 30, 2016
commit 9735e201bbfd0fefedcfaf5ad86c5a4716ee6392
1 change: 1 addition & 0 deletions python/MANIFEST.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ recursive-include deps/data *
recursive-include deps/examples *.py
recursive-include lib *.zip
include README.md
include deps/licenses/*.txt
11 changes: 11 additions & 0 deletions python/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@
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")
Copy link
Contributor

@holdenk holdenk Dec 4, 2016

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@lins05 lins05 Dec 4, 2016

Choose a reason for hiding this comment

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

Done. Good catch!

LICENSES_TARGET = os.path.join(TEMP_PATH, "licenses")

data_files = glob.glob(os.path.join(LICENSES_PATH, "*"))
Copy link
Contributor

@holdenk holdenk Dec 2, 2016

Choose a reason for hiding this comment

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

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

Copy link
Member

@srowen srowen Dec 2, 2016

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@holdenk holdenk Dec 2, 2016

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@lins05 lins05 Dec 4, 2016

Choose a reason for hiding this comment

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

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


# Check and see if we are under the spark path in which case we need to build the symlink farm.
# This is important because we only want to build the symlink farm while under Spark otherwise we
Expand Down Expand Up @@ -117,12 +120,14 @@ def _supports_symlinks():
os.symlink(SCRIPTS_PATH, SCRIPTS_TARGET)
os.symlink(EXAMPLES_PATH, EXAMPLES_TARGET)
os.symlink(DATA_PATH, DATA_TARGET)
os.symlink(LICENSES_PATH, LICENSES_TARGET)
else:
# For windows fall back to the slower copytree
copytree(JARS_PATH, JARS_TARGET)
copytree(SCRIPTS_PATH, SCRIPTS_TARGET)
copytree(EXAMPLES_PATH, EXAMPLES_TARGET)
copytree(DATA_PATH, DATA_TARGET)
copytree(LICENSES_PATH, LICENSES_TARGET)
else:
# If we are not inside of SPARK_HOME verify we have the required symlink farm
if not os.path.exists(JARS_TARGET):
Expand Down Expand Up @@ -166,21 +171,25 @@ def _supports_symlinks():
'pyspark.python.pyspark',
'pyspark.python.lib',
'pyspark.data',
'pyspark.licenses',
'pyspark.examples.src.main.python'],
include_package_data=True,
package_dir={
'pyspark.jars': 'deps/jars',
'pyspark.bin': 'deps/bin',
'pyspark.python.lib': 'lib',
'pyspark.data': 'deps/data',
'pyspark.licenses': 'deps/licenses',
'pyspark.examples.src.main.python': 'deps/examples',
},
package_data={
'pyspark.jars': ['*.jar'],
'pyspark.bin': ['*'],
'pyspark.python.lib': ['*.zip'],
'pyspark.data': ['*'],
'pyspark.licenses': ['*.txt'],
'pyspark.examples.src.main.python': ['*.py', '*/*.py']},
data_files=[('', data_files)],
Copy link
Contributor

@holdenk holdenk Dec 2, 2016

Choose a reason for hiding this comment

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

As mentioned above - do we really need this directive?

scripts=scripts,
license='http://www.apache.org/licenses/LICENSE-2.0',
install_requires=['py4j==0.10.4'],
Expand Down Expand Up @@ -210,9 +219,11 @@ def _supports_symlinks():
os.remove(os.path.join(TEMP_PATH, "bin"))
os.remove(os.path.join(TEMP_PATH, "examples"))
os.remove(os.path.join(TEMP_PATH, "data"))
os.remove(os.path.join(TEMP_PATH, "licenses"))
else:
rmtree(os.path.join(TEMP_PATH, "jars"))
rmtree(os.path.join(TEMP_PATH, "bin"))
rmtree(os.path.join(TEMP_PATH, "examples"))
rmtree(os.path.join(TEMP_PATH, "data"))
rmtree(os.path.join(TEMP_PATH, "licenses"))
os.rmdir(TEMP_PATH)