tvalentyn commented on code in PR #23879:
URL: https://github.com/apache/beam/pull/23879#discussion_r1007577272
##########
sdks/python/apache_beam/transforms/util_test.py:
##########
@@ -1063,6 +1065,45 @@ def test_tostring_kvs_empty_delimeter(self):
assert_that(result, equal_to(["one1", "two2"]))
+class LogElementsTest(unittest.TestCase):
+ @pytest.fixture(scope="function")
+ def _capture_stdout_log(request, capsys):
+ with TestPipeline() as p:
+ result = (p | beam.Create([
+ TimestampedValue(
+ "event",
+ datetime(2022, 10, 1, 0, 0, 0, 0,
+ tzinfo=pytz.UTC).timestamp()),
+ TimestampedValue(
+ "event",
+ datetime(2022, 10, 2, 0, 0, 0, 0,
+ tzinfo=pytz.UTC).timestamp()),
+ ]) \
Review Comment:
nit: it's generally discouraged to use line continuation symbol. It should
be not necessary here given you have parentheses, which ensure statement
continuation.
##########
sdks/python/apache_beam/transforms/util.py:
##########
@@ -1105,6 +1105,47 @@ def Iterables(delimiter=None):
Kvs = Iterables
+class LogElements(PTransform):
Review Comment:
please add this class also to `__all__` (line 93). this should also make
imports simpler thanks to
https://github.com/apache/beam/blob/feaa1a277b20ae555fe488000bcffebd8c18a9ed/sdks/python/apache_beam/transforms/__init__.py#L28
and
https://github.com/apache/beam/blob/feaa1a277b20ae555fe488000bcffebd8c18a9ed/sdks/python/apache_beam/__init__.py#L98
##########
sdks/python/apache_beam/transforms/util.py:
##########
@@ -1105,6 +1105,47 @@ def Iterables(delimiter=None):
Kvs = Iterables
+class LogElements(PTransform):
Review Comment:
let's also add typehints like in Reshuffle, see: line 772-773
##########
playground/backend/containers/python/Dockerfile:
##########
@@ -49,7 +49,6 @@ COPY --from=build /go/bin/server_python_backend
/opt/playground/backend/
COPY --from=build /go/src/playground/backend/configs
/opt/playground/backend/configs/
# Install Python Katas Utils
-COPY katas/log_elements.py /go/src/katas/
Review Comment:
A lot of playground examples have:
`from log_elements import LogElements`. These statements would stop working,
so we should either look how to replace them or for a drop-in replacement keep
katas/log_elements.py which will simply import LogElements from util.py
--
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]
