github-actions[bot] commented on code in PR #40741:
URL: https://github.com/apache/beam/pull/40741#discussion_r4354968200
##########
sdks/python/apache_beam/testing/benchmarks/tpcds/tpcds_launcher.py:
##########
@@ -240,7 +240,7 @@ def run_single_query(
elapsed = time.monotonic() - t0
end_dt = datetime.datetime.now(datetime.timezone.utc)
err_str = str(exc)
- first_line = err_str.splitlines()[0] if err_str else repr(exc)
+ first_line = err_str.splitlines()[0]
Review Comment:
**🟡 Medium**: If `exc` has an empty string message (e.g., `RuntimeError()`),
`err_str` will be `""` and `err_str.splitlines()` will return an empty list
`[]`, causing `err_str.splitlines()[0]` to raise an `IndexError` inside the
exception handler. Restoring the fallback guard prevents an unhandled crash
during error reporting.
```suggestion
first_line = err_str.splitlines()[0] if err_str else repr(exc)
```
##########
sdks/python/apache_beam/testing/benchmarks/tpcds/tpcds_queries.py:
##########
@@ -34,22 +34,22 @@
"3", "7", "13", "15", "19", "26", "42", "43", "48", "52", "55", "91", "96"
]
-# The 55 TPC-DS queries supported by Beam Calcite SQL (returning > 0 rows).
-# Together with KNOWN_FAILING_QUERIES (48 queries), this partitions all 103
TPC-DS queries.
+# TPC-DS queries supported by Beam Calcite SQL (returning > 0 rows).
+# Together with KNOWN_FAILING_QUERIES, this partitions all 103 TPC-DS queries.
# yapf: disable
SUPPORTED_QUERIES: List[str] = [
"1", "3", "4", "7", "9", "10", "11", "12", "15", "17",
"19", "20", "21", "26", "31", "32", "33", "34", "35", "38",
"40", "42", "43", "45", "46", "49", "50", "52", "53", "55",
"56", "59", "60", "61", "63", "65", "66", "68", "69", "71",
"73", "74", "75", "76", "79", "82", "84", "87", "88", "89",
- "92", "93", "96", "97", "99",
+ "90", "92", "93", "96", "97", "98", "99",
]
# yapf: enable
-# The 48 known failing TPC-DS queries on Beam Calcite SQL with their expected
error substring.
+# Known failing TPC-DS queries on Beam Calcite SQL with their expected error
substring.
# Used by `--check_regression` and
`:sdks:python:apache_beam:testing:benchmarks:tpcds:checkAllQueries`
-# to verify that all 55 SUPPORTED_QUERIES succeed on PrismRunner and that the
remaining 48 known
+# to verify that all SUPPORTED_QUERIES succeed on PrismRunner and that the
remaining 48 known
Review Comment:
**🟢 Low**: Nit: Since the hardcoded counts were removed from the surrounding
comments so they don't go stale as queries are enabled (and
`KNOWN_FAILING_QUERIES` now has 46 entries rather than 48), consider removing
`48` here as well.
```suggestion
# to verify that all SUPPORTED_QUERIES succeed on PrismRunner and that the
remaining known
```
--
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]
