Polber commented on code in PR #30005:
URL: https://github.com/apache/beam/pull/30005#discussion_r1464072499
##########
sdks/python/apache_beam/yaml/yaml_provider.py:
##########
@@ -731,14 +731,46 @@ def _parse_window_spec(spec):
return beam.WindowInto(window_fn)
@staticmethod
- def log_for_testing():
+ def log_for_testing(
+ level: Optional[str] = 'INFO', prefix: Optional[str] = ''):
"""Logs each element of its input PCollection.
The output of this transform is a copy of its input for ease of use in
chain-style pipelines.
+
+ Args:
+ level: one of ERROR, INFO, or DEBUG, mapped to a corresponding
+ language-specific logging level
+ prefix: an optional identifier that will get prepended to the element
+ being logged
"""
+ # Keeping this simple to be language agnostic.
+ # The intent is not to develop a logging library (and users can always do)
+ # their own mappings to get fancier output.
+ log_levels = {
+ 'ERROR': logging.error,
+ 'INFO': logging.info,
+ 'DEBUG': logging.debug,
+ }
Review Comment:
Why not go ahead and include all the levels (WARNING, FATAL, OFF, etc.)?
Same above
##########
sdks/python/apache_beam/yaml/yaml_provider.py:
##########
@@ -731,14 +731,46 @@ def _parse_window_spec(spec):
return beam.WindowInto(window_fn)
@staticmethod
- def log_for_testing():
+ def log_for_testing(
+ level: Optional[str] = 'INFO', prefix: Optional[str] = ''):
Review Comment:
`prefix` could default to name of transform (i.e. LogForTesting) or maybe
even always put that as part of the log string and append this `prefix`
(defaulting to empty string) to that (i.e. `INFO:root:LogForTesting` or
`INFO:root:LogForTesting:prefix`) to explicitly show where the log is coming
from.
Similar for the Java version, although I'm not sure what is included in the
log string by default in that case (perhaps the source class is already
included)
##########
sdks/python/apache_beam/yaml/yaml_provider.py:
##########
@@ -731,14 +731,46 @@ def _parse_window_spec(spec):
return beam.WindowInto(window_fn)
@staticmethod
- def log_for_testing():
+ def log_for_testing(
+ level: Optional[str] = 'INFO', prefix: Optional[str] = ''):
"""Logs each element of its input PCollection.
The output of this transform is a copy of its input for ease of use in
chain-style pipelines.
+
+ Args:
+ level: one of ERROR, INFO, or DEBUG, mapped to a corresponding
+ language-specific logging level
+ prefix: an optional identifier that will get prepended to the element
+ being logged
"""
+ # Keeping this simple to be language agnostic.
+ # The intent is not to develop a logging library (and users can always do)
+ # their own mappings to get fancier output.
+ log_levels = {
+ 'ERROR': logging.error,
+ 'INFO': logging.info,
+ 'DEBUG': logging.debug,
+ }
+ if level not in log_levels:
+ raise ValueError(
+ f'Unknown long level {level} not in {list(log_levels.keys())}')
Review Comment:
```suggestion
f'Unknown log level {level} not in {list(log_levels.keys())}')
```
--
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]
