damccorm commented on code in PR #28948: URL: https://github.com/apache/beam/pull/28948#discussion_r1357135139
########## CHANGES.md: ########## @@ -79,6 +79,7 @@ ## Bugfixes * Fixed "Desired bundle size 0 bytes must be greater than 0" in Java SDK's BigtableIO.BigtableSource when you have more cores than bytes to read (Java) [#28793](https://github.com/apache/beam/issues/28793). +* `watch_file_pattern` arg of the [RunInference](https://github.com/apache/beam/blob/104c10b3ee536a9a3ea52b4dbf62d86b669da5d9/sdks/python/apache_beam/ml/inference/base.py#L997) arg had no effect prior to 2.52.0. To use the behavior of arg `watch_file_pattern` prior to 2.52.0, follow the documentation at https://beam.apache.org/documentation/ml/side-input-updates/ and use `WatchFilePattern` PTransform as a SideInput. ([#28948](https://github.com/apache/beam/pulls/28948)) Review Comment: Could you please add a test to show this works ########## sdks/python/apache_beam/ml/inference/base.py: ########## @@ -1131,7 +1131,7 @@ def expand( self._inference_args, beam.pvalue.AsSingleton( self._model_metadata_pcoll, - ) if self._enable_side_input_loading else None).with_resource_hints( + ) if self._model_metadata_pcoll else None).with_resource_hints( Review Comment: `_enable_side_input_loading` is used elsewhere; can we update those references as well? Might make sense to remove it as a class variable and just make it a local variable that we can use in `expand` -- 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]
