shaheeramjad commented on code in PR #37423:
URL: https://github.com/apache/beam/pull/37423#discussion_r2740228152
##########
sdks/python/apache_beam/transforms/enrichment_handlers/cloudsql.py:
##########
@@ -395,11 +395,11 @@ def _execute_query(
return data
except Exception as e:
transaction.rollback()
- raise RuntimeError(f"Database operation failed: {e}")
+ raise RuntimeError(f"Database operation failed: {e}") from e
Review Comment:
With from e, the cause is in __cause__ and the traceback, so we don’t need
to include the exception in the message.
I kept it in because in many environments only the top-level exception
message is logged or shown (e.g. in alerts or dashboards). In those cases,
"Database operation failed" alone doesn’t tell you the real error, while
f"Database operation failed: {e}" stays useful. It’s also a common Python
pattern to add context and still include the original error in the wrapper
message.
If you prefer to rely on the chain and keep messages minimal, I’m happy to
change to e.g. raise RuntimeError("Database operation failed") from e and
remove {e} from the message. Happy to follow whatever convention you prefer for
this project.
--
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]
