dpgaspar commented on a change in pull request #8163: [SQLLab] Refactor sql
json endpoint
URL:
https://github.com/apache/incubator-superset/pull/8163#discussion_r321982531
##########
File path: superset/views/core.py
##########
@@ -2557,145 +2619,142 @@ def validate_sql_json(self):
)
return json_error_response(f"{msg}")
+ def _sql_json_async(self, session, rendered_query, query: Query) -> str:
+ """
+ Send SQL JSON query to celery workers
+
+ :param session: SQLAlchemy session object
+ :param rendered_query: the rendered query to perform by workers
+ :param query: The query (SQLAlchemy) object
+ :return: String JSON response
+ """
+ logging.info(f"Query {query.id}: Running query on a Celery worker")
+ # Ignore the celery future object and the request may time out.
+ try:
+ sql_lab.get_sql_results.delay(
+ query.id,
+ rendered_query,
+ return_results=False,
+ store_results=not query.select_as_cta,
+ user_name=g.user.username if g.user else None,
+ start_time=now_as_float(),
+ )
+ except Exception as e:
+ logging.exception(f"Query {query.id}: {e}")
+ msg = _(
+ "Failed to start remote query on a worker. "
+ "Tell your administrator to verify the availability of "
+ "the message queue."
+ )
+ query.status = QueryStatus.FAILED
+ query.error_message = msg
+ session.commit()
+ return json_error_response("{}".format(msg))
+ resp = json_success(
+ json.dumps(
+ {"query": query.to_dict()},
+ default=utils.json_int_dttm_ser,
+ ignore_nan=True,
+ ),
+ status=202,
+ )
+ session.commit()
+ return resp
+
+ def _sql_json_sync(self, session, rendered_query, query: Query) -> str:
+ """
+ Execute SQL query (sql json)
+
+ :param rendered_query: The rendered query (included templates)
+ :param query: The query SQL (SQLAlchemy) object
+ :return: String JSON response
+ """
+ try:
+ timeout = config.get("SQLLAB_TIMEOUT")
+ timeout_msg = f"The query exceeded the {timeout} seconds timeout."
+ with utils.timeout(seconds=timeout, error_message=timeout_msg):
+ # pylint: disable=no-value-for-parameter
+ data = sql_lab.get_sql_results(
+ query.id,
+ rendered_query,
+ return_results=True,
+ user_name=g.user.username if g.user else None,
+ )
+
+ payload = json.dumps(
+ apply_display_max_row_limit(data),
+ default=utils.pessimistic_json_iso_dttm_ser,
+ ignore_nan=True,
+ encoding=None,
+ )
+ except Exception as e:
+ logging.exception(f"Query {query.id}: {e}")
+ return json_error_response(f"{{e}}")
+ if data.get("status") == QueryStatus.FAILED:
+ return json_error_response(payload=data)
+ return json_success(payload)
+
@has_access_api
@expose("/sql_json/", methods=["POST", "GET"])
@event_logger.log_this
def sql_json(self):
"""Runs arbitrary sql and returns and json"""
- async_ = request.form.get("runAsync") == "true"
- sql = request.form.get("sql")
- database_id = request.form.get("database_id")
- schema = request.form.get("schema") or None
- template_params = json.loads(request.form.get("templateParams") or
"{}")
- limit = int(request.form.get("queryLimit", 0))
- if limit < 0:
- logging.warning(
- "Invalid limit of {} specified. Defaulting to max
limit.".format(limit)
- )
- limit = 0
- limit = limit or app.config.get("SQL_MAX_ROW")
-
+ # Collect Values
+ params = SQLJsonParams()
session = db.session()
- mydb = session.query(models.Database).filter_by(id=database_id).first()
+ # Get and Validate database
+ mydb =
session.query(models.Database).filter_by(id=params.database_id).first()
if not mydb:
- json_error_response("Database with id {} is
missing.".format(database_id))
+ json_error_response(
+ "Database with id {} is missing.".format(params.database_id)
+ )
- rejected_tables = security_manager.rejected_tables(sql, mydb, schema)
+ # Check if it's a rejected datasource
+ rejected_tables = security_manager.rejected_tables(
+ params.sql, mydb, params.schema
+ )
if rejected_tables:
return json_error_response(
security_manager.get_table_access_error_msg(rejected_tables),
link=security_manager.get_table_access_link(rejected_tables),
status=403,
)
+ # ?
Review comment:
`session.commit()` previous code had this commit, do we really need it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
