[
https://issues.apache.org/jira/browse/BEAM-13685?focusedWorklogId=711627&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-711627
]
ASF GitHub Bot logged work on BEAM-13685:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 19/Jan/22 20:17
Start Date: 19/Jan/22 20:17
Worklog Time Spent: 10m
Work Description: rohdesamuel commented on a change in pull request
#16555:
URL: https://github.com/apache/beam/pull/16555#discussion_r788104377
##########
File path:
sdks/python/apache_beam/runners/interactive/caching/streaming_cache.py
##########
@@ -247,8 +247,11 @@ def __init__(
self._sample_resolution_sec = sample_resolution_sec
self._is_cache_complete = is_cache_complete
+ from apache_beam.runners.interactive import interactive_beam as ib
if cache_dir:
self._cache_dir = cache_dir
+ elif ib.options.specified_cache_dir:
+ self._cache_dir = ib.options.specified_cache_dir
Review comment:
I like to call code like this "spooky action at a distance". In general,
configuration should be passed in through the constructor. This makes it
obvious as to what settings the object needs. Accessing a global variable
within the constructor esp. with an existing cache_dir argument can make it
confusing for maintainers. I recommend moving this logic to where the
StreamingCache and CacheManager are created and the cache_dir parameter is
passed.
##########
File path: sdks/python/apache_beam/runners/interactive/interactive_beam.py
##########
@@ -220,6 +220,24 @@ def display_timezone(self, value):
"""
self._display_timezone = value
+ @property
+ def specified_cache_dir(self):
+ """The cache directory specified by the user.
+
+ Defaults to None.
+ """
+ return self._specified_cache_dir
+
+ @specified_cache_dir.setter
+ def specified_cache_dir(self, value):
+ """Sets the cache directory.
+
+ Defaults to None.
+
+ Example::
+ interactive_beam.options.cache_dir = "gs://"
Review comment:
Please fix: the pipeline option in the example ("cache_dir") does not
match the actual pipeline option ("specified_cache_dir").
--
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]
Issue Time Tracking
-------------------
Worklog Id: (was: 711627)
Time Spent: 1h (was: 50m)
> Enable users to specify cache directory under Interactive Beam options
> ----------------------------------------------------------------------
>
> Key: BEAM-13685
> URL: https://issues.apache.org/jira/browse/BEAM-13685
> Project: Beam
> Issue Type: Task
> Components: runner-py-interactive
> Reporter: Victor Chen
> Assignee: Victor Chen
> Priority: P2
> Time Spent: 1h
> Remaining Estimate: 0h
>
> - Adds specified_cache_dir option under interactive_beam.py and
> interactive_options.py
> - Integrates new specified_cache_dir from interactive_beam.py into
> cache_manager.py and streaming_cache.py
> - Includes unit tests under cache_manager_test.py and streaming_cache_test.py
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
