VictorPlusC commented on a change in pull request #16601:
URL: https://github.com/apache/beam/pull/16601#discussion_r794969238
##########
File path:
sdks/python/apache_beam/runners/interactive/interactive_environment.py
##########
@@ -357,19 +363,47 @@ def get_cache_manager(self, pipeline,
create_if_absent=False):
given pipeline. If the pipeline is absent from the environment while
create_if_absent is True, creates and returns a new file based cache
manager for the pipeline."""
+ if self._is_in_ipython:
+ warnings.filterwarnings(
+ 'ignore',
+ 'options is deprecated since First stable release. References to '
+ '<pipeline>.options will not be supported',
+ category=DeprecationWarning)
+
cache_manager = self._cache_managers.get(str(id(pipeline)), None)
+ if isinstance(pipeline, Pipeline):
+ if hasattr(pipeline.runner, '_underlying_runner'):
Review comment:
I've replaced the second line with isinstance(pipeline.runner,
InteractiveRunner), but I've currently kept the initial check to make sure that
the pipeline argument is a Pipeline object because we have several tests in
interactive_environment_test.py and in recording_manager_test.py that use this
function with the pipeline argument as a string. It seems that more than half
of the tests in recording_manager_test.py run into an issue where the
`pipeline.runner` code will error out as a string is being passed into this
function in those tests.
Do you think we should leave it as is, or should we fix the unit tests that
are failing in this case?
--
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]
