Skip to content
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

Use lazy_loader for pyspark #168

Merged
merged 5 commits into from
Aug 28, 2020
Merged

Use lazy_loader for pyspark #168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d4b47cf
Remove dependence on pyspark for non-spark notebooks
Aug 26, 2020
0bfcc3d
Use lazy_loader for pyspark
Aug 27, 2020
ccdc807
lint
Aug 27, 2020
b2f7cbf
Merge branch 'master' into notebook
akhurana001 Aug 27, 2020
5d09fc7
lint
Aug 27, 2020
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
2 changes: 1 addition & 1 deletion flytekit/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

import flytekit.plugins # noqa: F401

__version__ = "0.12.1"
__version__ = "0.12.2"
8 changes: 4 additions & 4 deletions flytekit/contrib/notebook/helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import os as _os

import six as _six
from pyspark import SparkConf, SparkContext

from flytekit.common.types.helpers import pack_python_std_map_to_literal_map as _packer
from flytekit.contrib.notebook.supported_types import notebook_types_map as _notebook_types_map
from flytekit.plugins import pyspark as _pyspark
Copy link
Contributor

Choose a reason for hiding this comment

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

this will work because only when we access pyspark.x will it load the real module?

Copy link
Contributor Author

@akhurana001 akhurana001 Aug 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, that's my understanding of lazy_loading here: https://github.com/lyft/flytekit/blob/master/flytekit/plugins/__init__.py#L5 . Have tested, it works.



def record_outputs(outputs=None):
Expand Down Expand Up @@ -35,9 +35,9 @@ def get_spark_context(spark_conf):
# We run in cluster-mode in Flyte.
# Ref https://github.com/lyft/flyteplugins/blob/master/go/tasks/v1/flytek8s/k8s_resource_adds.go#L46
if "FLYTE_INTERNAL_EXECUTION_ID" in _os.environ:
return SparkContext()
return _pyspark.SparkContext()

# Add system spark-conf for local/notebook based execution.
spark_conf.add(("spark.master", "local"))
conf = SparkConf().setAll(spark_conf)
return SparkContext(conf=conf)
conf = _pyspark.SparkConf().setAll(spark_conf)
return _pyspark.SparkContext(conf=conf)
1 change: 1 addition & 0 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"urllib3>=1.22,<2.0.0",
"wrapt>=1.0.0,<2.0.0",
"papermill>=1.2.0",
"ipykernel>=5.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

@akhurana001 akhurana001 Aug 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

See the description. Still not sure why it worked before but adding this does resolve the error I was seeing while testing: jupyter/notebook#1558

],
extras_require=extras_require,
scripts=[
Expand Down