You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/01/27 20:08:12 UTC
[GitHub] [superset] davemulford opened a new pull request #18207: fix(postgresql): fix TEIID error when getting postgresql cancellation token
davemulford opened a new pull request #18207:
URL: https://github.com/apache/superset/pull/18207
Using TEIID virtual databases (VDBs) in Superset requires the use of the postgres connector. In a previous update, cancelation tokens were added to allow a query to be stopped on the database server. This is a good thing, however it introduced a bug that disallowed the use of TEIID VDBs.
This change wraps the `SELECT pg_backend_pid()` query in a try-catch phrase, and returns `None` upon exception.
The error encountered in the Superset SQL Editor is: _The pg_backend_pid() function has unknown form_.
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro commented on a change in pull request #18207: fix(postgresql): fix TEIID error when getting postgresql cancellation token
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #18207:
URL: https://github.com/apache/superset/pull/18207#discussion_r794711951
##########
File path: superset/db_engine_specs/postgres.py
##########
@@ -289,9 +289,12 @@ def get_cancel_query_id(cls, cursor: Any, query: Query) -> Optional[str]:
:param query: Query instance
:return: Postgres PID
"""
- cursor.execute("SELECT pg_backend_pid()")
- row = cursor.fetchone()
- return row[0]
+ try:
+ cursor.execute("SELECT pg_backend_pid()")
+ row = cursor.fetchone()
+ return row[0]
+ except Exception:
Review comment:
> I've updated the PR comment with the error being raised. It's a `superset.exceptions.SupersetErrorsException`. Would you like me to catch that instead of the generic exception?
Really? I would have thought it's a psycopg2 or at least a SQLAlchemy exception, as I assume it's coming from the cursor call. But yeah, let's make it as specific as possible if it's not too much trouble.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] davemulford commented on a change in pull request #18207: fix(postgresql): fix TEIID error when getting postgresql cancellation token
Posted by GitBox <gi...@apache.org>.
davemulford commented on a change in pull request #18207:
URL: https://github.com/apache/superset/pull/18207#discussion_r794668845
##########
File path: superset/db_engine_specs/postgres.py
##########
@@ -289,9 +289,12 @@ def get_cancel_query_id(cls, cursor: Any, query: Query) -> Optional[str]:
:param query: Query instance
:return: Postgres PID
"""
- cursor.execute("SELECT pg_backend_pid()")
- row = cursor.fetchone()
- return row[0]
+ try:
+ cursor.execute("SELECT pg_backend_pid()")
+ row = cursor.fetchone()
+ return row[0]
+ except Exception:
Review comment:
I've updated the PR comment with the error being raised. It's a `superset.exceptions.SupersetErrorsException`. Would you like me to catch that instead of the generic exception?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro commented on a change in pull request #18207: fix(postgresql): fix TEIID error when getting postgresql cancellation token
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #18207:
URL: https://github.com/apache/superset/pull/18207#discussion_r794530449
##########
File path: superset/db_engine_specs/postgres.py
##########
@@ -289,9 +289,12 @@ def get_cancel_query_id(cls, cursor: Any, query: Query) -> Optional[str]:
:param query: Query instance
:return: Postgres PID
"""
- cursor.execute("SELECT pg_backend_pid()")
- row = cursor.fetchone()
- return row[0]
+ try:
+ cursor.execute("SELECT pg_backend_pid()")
+ row = cursor.fetchone()
+ return row[0]
+ except Exception:
Review comment:
Minor improvement proposal: is there a more specific exception type that we could be catching here?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] davemulford commented on a change in pull request #18207: fix(postgresql): fix TEIID error when getting postgresql cancellation token
Posted by GitBox <gi...@apache.org>.
davemulford commented on a change in pull request #18207:
URL: https://github.com/apache/superset/pull/18207#discussion_r794619812
##########
File path: superset/db_engine_specs/postgres.py
##########
@@ -289,9 +289,12 @@ def get_cancel_query_id(cls, cursor: Any, query: Query) -> Optional[str]:
:param query: Query instance
:return: Postgres PID
"""
- cursor.execute("SELECT pg_backend_pid()")
- row = cursor.fetchone()
- return row[0]
+ try:
+ cursor.execute("SELECT pg_backend_pid()")
+ row = cursor.fetchone()
+ return row[0]
+ except Exception:
Review comment:
I can double-check and make the change. Thanks for the review!
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org