KevinGG commented on a change in pull request #16555:
URL: https://github.com/apache/beam/pull/16555#discussion_r789153119



##########
File path: 
sdks/python/apache_beam/runners/interactive/interactive_environment.py
##########
@@ -359,10 +359,22 @@ def get_cache_manager(self, pipeline, 
create_if_absent=False):
     manager for the pipeline."""
     cache_manager = self._cache_managers.get(str(id(pipeline)), None)
     if not cache_manager and create_if_absent:
-      cache_dir = tempfile.mkdtemp(
-          suffix=str(id(pipeline)),
-          prefix='it-',
-          dir=os.environ.get('TEST_TMPDIR', None))
+      from apache_beam.runners.interactive import interactive_beam as ib
+      if ib.options.cache_root:

Review comment:
       Can you please add a TODO here too to handle the case when the path is 
started with "gs://"? You may now just raise a ValueError about GCS path not 
supported.

##########
File path: sdks/python/apache_beam/runners/interactive/interactive_beam.py
##########
@@ -220,6 +220,27 @@ def display_timezone(self, value):
     """
     self._display_timezone = value
 
+  @property
+  def cache_root(self):
+    """The cache directory specified by the user.
+
+    Defaults to None.
+    """
+    return self._cache_root
+
+  @cache_root.setter
+  def cache_root(self, value):
+    """Sets the cache directory.
+
+    Defaults to None.
+
+    Example of local directory usage::
+      interactive_beam.options.cache_root = "/Users/username/my/cache/dir"
+
+    Example of GCS directory usage::

Review comment:
       Let's mark this docstring as not supported yet or delete it from the 
module since it's not supported yet.

##########
File path: sdks/python/apache_beam/runners/interactive/background_caching_job.py
##########
@@ -231,9 +231,21 @@ def has_source_to_cache(user_pipeline):
                       streaming_cache.StreamingCache):
 
       file_based_cm = ie.current_env().get_cache_manager(user_pipeline)
+      cache_dir = file_based_cm._cache_dir
+      from apache_beam.runners.interactive import interactive_beam as ib
+      if ib.options.cache_root:
+        _LOGGER.warning(
+            'Interactive Beam has detected a set value for the cache_root '
+            'option. Please note: existing cache managers will not have '

Review comment:
       You may move the warning to the setter of the property in 
interactive_beam module. Then you only need to code this logging once when the 
value is modified.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to