michael-s-molina commented on code in PR #20683:
URL: https://github.com/apache/superset/pull/20683#discussion_r922232391
##########
superset/datasets/api.py:
##########
@@ -810,7 +822,15 @@ def samples(self, pk: int) -> Response:
"""
try:
force = parse_boolean_string(request.args.get("force"))
- rv = SamplesDatasetCommand(pk, force).run()
+ page = request.args.get("page")
+ per_page = request.args.get("per_page")
+ rv = SamplesDatasetCommand(
+ pk,
+ force,
Review Comment:
```suggestion
model_id=pk,
force=force,
```
##########
superset/datasets/commands/samples.py:
##########
@@ -30,40 +29,79 @@
DatasetSamplesFailedError,
)
from superset.datasets.dao import DatasetDAO
+from superset.datasets.schemas import DatasetSamplesQuerySchema
from superset.exceptions import SupersetSecurityException
-from superset.utils.core import QueryStatus
-
-logger = logging.getLogger(__name__)
+from superset.utils.core import DatasourceDict, QueryStatus
class SamplesDatasetCommand(BaseCommand):
- def __init__(self, model_id: int, force: bool):
+ def __init__(
+ self,
+ model_id: int,
+ force: bool,
+ *,
Review Comment:
```suggestion
```
##########
superset/datasets/commands/samples.py:
##########
@@ -78,3 +116,24 @@ def validate(self) -> None:
security_manager.raise_for_ownership(self._model)
except SupersetSecurityException as ex:
raise DatasetForbiddenError() from ex
+
+ @staticmethod
+ def get_limit_clause(
+ page: Optional[int], per_page: Optional[int]
+ ) -> Dict[str, int]:
+ samples_row_limit = app.config.get("SAMPLES_ROW_LIMIT", 1000)
+ limit = samples_row_limit
+ offset = 0
+
+ if isinstance(page, int) and isinstance(per_page, int):
Review Comment:
`isinstance` fails because `page` and `per_page` are strings in the URL, so
they are being ignored.
##########
superset/datasets/commands/samples.py:
##########
@@ -30,40 +29,79 @@
DatasetSamplesFailedError,
)
from superset.datasets.dao import DatasetDAO
+from superset.datasets.schemas import DatasetSamplesQuerySchema
from superset.exceptions import SupersetSecurityException
-from superset.utils.core import QueryStatus
-
-logger = logging.getLogger(__name__)
+from superset.utils.core import DatasourceDict, QueryStatus
class SamplesDatasetCommand(BaseCommand):
- def __init__(self, model_id: int, force: bool):
+ def __init__(
+ self,
+ model_id: int,
+ force: bool,
+ *,
+ payload: Optional[DatasetSamplesQuerySchema] = None,
+ page: Optional[int] = None,
+ per_page: Optional[int] = None,
+ ):
self._model_id = model_id
self._force = force
self._model: Optional[SqlaTable] = None
+ self._payload = payload
+ self._page = page
+ self._per_page = per_page
def run(self) -> Dict[str, Any]:
self.validate()
- if not self._model:
- raise DatasetNotFoundError()
+ limit_clause = self.get_limit_clause(self._page, self._per_page)
+ self._model = cast(SqlaTable, self._model)
+ datasource: DatasourceDict = {
+ "type": self._model.type,
+ "id": self._model.id,
+ }
- qc_instance = QueryContextFactory().create(
- datasource={
- "type": self._model.type,
- "id": self._model.id,
- },
- queries=[{}],
+ # constructing samples query
+ samples_instance = QueryContextFactory().create(
+ datasource=datasource,
+ queries=[
+ {**self._payload, **limit_clause} if self._payload else
limit_clause
+ ],
result_type=ChartDataResultType.SAMPLES,
force=self._force,
)
- results = qc_instance.get_payload()
+
+ # constructing count(*) query
+ count_star_payload = {
Review Comment:
We should consider the filters for the count as well because the pagination
is calculated considering them.
Right now, if I do a query with:
```
{"filters": [{"col": "gender", "op": "==", "val": "boy"}]}
```
I get the total count in `dataset_count_star` without the filters.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
