damccorm commented on code in PR #30001:
URL: https://github.com/apache/beam/pull/30001#discussion_r1457539010


##########
sdks/python/apache_beam/io/requestresponse.py:
##########
@@ -278,14 +278,14 @@ def __init__(
       timeout (float): timeout value in seconds to wait for response from API.
       should_backoff (~apache_beam.io.requestresponse.ShouldBackOff):
         (Optional) provides methods for backoff.
-      repeater (~apache_beam.io.requestresponse.Repeater): (Optional)
-        provides methods to repeat requests to API.
+      repeater (~apache_beam.io.requestresponse.Repeater): provides methods to
+        repeat requests to API.
       cache_reader (~apache_beam.io.requestresponse.CacheReader): (Optional)
         provides methods to read external cache.
       cache_writer (~apache_beam.io.requestresponse.CacheWriter): (Optional)
         provides methods to write to external cache.
       throttler (~apache_beam.io.requestresponse.PreCallThrottler):
-        (Optional) provides methods to pre-throttle a request.
+        provides methods to pre-throttle a request.

Review Comment:
   Would probably be good to explain the default behavior for this and the 
repeater



##########
sdks/python/apache_beam/transforms/enrichment_handlers/bigtable.py:
##########
@@ -63,32 +65,43 @@ class EnrichWithBigTable(EnrichmentSourceHandler[beam.Row, 
beam.Row]):
       to use as `row_key` for BigTable querying.
     row_filter: a ``:class:`google.cloud.bigtable.row_filters.RowFilter``` to
       filter data read with ``read_row()``.
+      Defaults to `CellsColumnLimitFilter(1)`.
+    app_profile_id (str): App profile ID to use for BigTable.

Review Comment:
   Would be good to link to https://cloud.google.com/bigtable/docs/app-profiles 
here



##########
sdks/python/apache_beam/transforms/enrichment_handlers/bigtable.py:
##########
@@ -81,7 +81,7 @@ def __init__(
       table_id: str,
       row_key: str,
       row_filter: Optional[RowFilter] = CellsColumnLimitFilter(1),
-      app_profile_id: str = None,
+      app_profile_id: str = "",

Review Comment:
   Looking at 
https://cloud.google.com/python/docs/reference/bigtable/latest/google.cloud.bigtable.table.Table
 I think None is actually the correct default



-- 
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]

Reply via email to