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/04/19 22:13:24 UTC
[GitHub] [superset] yourssvk opened a new pull request, #16326: fix: SQL Lab cancel query in Redshift database connection does not wo…
yourssvk opened a new pull request, #16326:
URL: https://github.com/apache/superset/pull/16326
…rk #16325
### 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 -->
- [x] 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] eschutho closed pull request #16326: fix: SQL Lab cancel query in Redshift database connection does not wo…
Posted by GitBox <gi...@apache.org>.
eschutho closed pull request #16326: fix: SQL Lab cancel query in Redshift database connection does not wo…
URL: https://github.com/apache/superset/pull/16326
--
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] eschutho commented on a diff in pull request #16326: fix: SQL Lab cancel query in Redshift database connection does not wo…
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #16326:
URL: https://github.com/apache/superset/pull/16326#discussion_r906623858
##########
superset/db_engine_specs/redshift.py:
##########
@@ -15,13 +15,17 @@
# specific language governing permissions and limitations
# under the License.
import re
Review Comment:
```suggestion
import logging
import re
```
--
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] eschutho merged pull request #16326: fix: SQL Lab cancel query in Redshift database connection does not wo…
Posted by GitBox <gi...@apache.org>.
eschutho merged PR #16326:
URL: https://github.com/apache/superset/pull/16326
--
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] eschutho commented on a diff in pull request #16326: fix: SQL Lab cancel query in Redshift database connection does not wo…
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #16326:
URL: https://github.com/apache/superset/pull/16326#discussion_r907877352
##########
superset/db_engine_specs/redshift.py:
##########
@@ -14,14 +14,18 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+import logging
import re
-from typing import Any, Dict, Pattern, Tuple
+from typing import Any, Dict, Pattern, Tuple, Optional
Review Comment:
```suggestion
from typing import Any, Dict, Optional, Pattern, Tuple
```
--
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] eschutho commented on a diff in pull request #16326: fix: SQL Lab cancel query in Redshift database connection does not wo…
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #16326:
URL: https://github.com/apache/superset/pull/16326#discussion_r906591207
##########
superset/db_engine_specs/redshift.py:
##########
@@ -101,3 +105,39 @@ def _mutate_label(label: str) -> str:
:return: Conditionally mutated label
"""
return label.lower()
+
+ @classmethod
+ def get_cancel_query_id(cls, cursor: Any, query: Query) -> Optional[str]:
+ """
+ Get Redshift PID that will be used to cancel all other running
+ queries in the same session.
+
+ :param cursor: Cursor instance in which the query will be executed
+ :param query: Query instance
+ :return: Redshift PID
+ """
+ cursor.execute("SELECT pg_backend_pid()")
+ row = cursor.fetchone()
+ return row[0]
+
+ @classmethod
+ def cancel_query(cls, cursor: Any, query: Query, cancel_query_id: str) -> bool:
+ """
+ Cancel query in the underlying database.
+
+ :param cursor: New cursor instance to the db of the query
+ :param query: Query instance
+ :param cancel_query_id: Redshift PID
+ :return: True if query cancelled successfully, False otherwise
+ """
+ try:
+ logger.info("Killing Redshift PID:%s",str(cancel_query_id))
Review Comment:
```suggestion
logger.info("Killing Redshift PID:%s", str(cancel_query_id))
```
--
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] eschutho commented on a diff in pull request #16326: fix: SQL Lab cancel query in Redshift database connection does not wo…
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #16326:
URL: https://github.com/apache/superset/pull/16326#discussion_r906623882
##########
superset/db_engine_specs/redshift.py:
##########
@@ -15,13 +15,17 @@
# specific language governing permissions and limitations
# under the License.
import re
-from typing import Any, Dict, Pattern, Tuple
+import logging
Review Comment:
```suggestion
```
--
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] eschutho commented on pull request #16326: fix: SQL Lab cancel query in Redshift database connection does not wo…
Posted by GitBox <gi...@apache.org>.
eschutho commented on PR #16326:
URL: https://github.com/apache/superset/pull/16326#issuecomment-1107033552
@yourssvk sorry for the delay. It looks like there's just one linting error that's failing the tests. Can you try running pre-commit locally to fix it? There are instructions isn https://github.com/apache/superset/blob/master/CONTRIBUTING.md how to get pre-commit running.
--
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
Re: [PR] fix: SQL Lab cancel query in Redshift database connection does not wo… [superset]
Posted by "gaurav7261 (via GitHub)" <gi...@apache.org>.
gaurav7261 commented on PR #16326:
URL: https://github.com/apache/superset/pull/16326#issuecomment-1786796394
@yourssvk I'm getting empty query id while superset code calling `cancel_query_id = db_engine_spec.get_cancel_query_id(cursor, query)` in sql_lab.py with engine ReshiftSpec, while for postgres spec,I get queryid, so stop query is working fine in case of postgres but not in redshift, did some logging to verify, here is the log
for postgres
```15965
2023-10-31 08:48:29,251:INFO:superset.sql_lab:15965
<class 'superset.db_engine_specs.postgres.PostgresEngineSpec'>```
where 15965 is postgres query id but for redshift sql lab
```None
2023-10-31 08:44:28,331:INFO:superset.sql_lab:None
<class 'superset.db_engine_specs.redshift-bak.RedshiftEngineSpec'>```
it is returning null, my version is 2.1.1, is it known issue
--
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