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/11/22 11:04:40 UTC
[GitHub] [superset] mayurnewase opened a new pull request, #22196: fix: force data generation in csv reports
mayurnewase opened a new pull request, #22196:
URL: https://github.com/apache/superset/pull/22196
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
fixes: #22189
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
1. create table chart
2. setup csv report on that chart and enable `force cache while generating thumbnails`
3. update data in datasource without bursting cache
4. check csv in the report received, should contain new data
### 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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1029966009
##########
superset/charts/data/api.py:
##########
@@ -130,6 +135,7 @@ def get_data(self, pk: int) -> Response:
"format", ChartDataResultFormat.JSON
)
json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL)
+ json_body["force"] = request.args.get("force", False)
Review Comment:
if exists another deserialization helper on the json_body, the default value of `request.args.get("force")` should be None instead of `request.args.get("force", False)`, because the `request.args.get("force")` will always return `Optional[str]`.
--
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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032731414
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,59 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ # wrapping original class, so we can test output too.
+ with mock.patch(
+ "superset.charts.data.api.ChartDataCommand", wraps=ChartDataCommand
+ ) as command:
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+ data = json.loads(rv.data.decode("utf-8"))
+ query_context = command.call_args.args[0]
Review Comment:
we should verify the `is_cached` field in the response.
```Python
test_client.post(f"api/v1/chart/{chart.id}/data/?force=true")
# should through cache
rv = test_client.post(f"api/v1/chart/{chart.id}/data/?force=true")
assert rv.json["result"][0]["is_cached"] is None
test_client.post(f"api/v1/chart/{chart.id}/data")
# should get response from the cache
rv = test_client.post(f"api/v1/chart/{chart.id}/data/?force=true")
assert rv.json["result"][0]["is_cached"] is not None
```
--
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] mayurnewase commented on pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on PR #22196:
URL: https://github.com/apache/superset/pull/22196#issuecomment-1323517287
Should we also change `Ignore cache when generating screenshot` to `Ignore cache when generating report` as CSV, json do not need screenshots.
--
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] mayurnewase commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032726613
##########
superset/charts/data/api.py:
##########
@@ -130,6 +135,7 @@ def get_data(self, pk: int) -> Response:
"format", ChartDataResultFormat.JSON
)
json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL)
+ json_body["force"] = request.args.get("force", False)
Review Comment:
ok updated.
--
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] mayurnewase commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032732654
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,59 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ # wrapping original class, so we can test output too.
+ with mock.patch(
+ "superset.charts.data.api.ChartDataCommand", wraps=ChartDataCommand
+ ) as command:
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+ data = json.loads(rv.data.decode("utf-8"))
+ query_context = command.call_args.args[0]
Review Comment:
updated.
--
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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1029942672
##########
superset/charts/data/api.py:
##########
@@ -130,6 +135,7 @@ def get_data(self, pk: int) -> Response:
"format", ChartDataResultFormat.JSON
)
json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL)
+ json_body["force"] = request.args.get("force", False)
Review Comment:
In order to handle `false` as a string in the force, an explicit type conversion should be in here. for example:
```type
json_body["force"] = explict_bool_conversion(request.args.get("force"))
```
##########
superset/charts/data/api.py:
##########
@@ -130,6 +135,7 @@ def get_data(self, pk: int) -> Response:
"format", ChartDataResultFormat.JSON
)
json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL)
+ json_body["force"] = request.args.get("force", False)
Review Comment:
In order to handle `false` as a string in the force, an explicit type conversion should be in here. for example:
```Python
json_body["force"] = explict_bool_conversion(request.args.get("force"))
```
--
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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1029969250
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,59 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ # wrapping original class, so we can test output too.
+ with mock.patch(
+ "superset.charts.data.api.ChartDataCommand", wraps=ChartDataCommand
+ ) as command:
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+ data = json.loads(rv.data.decode("utf-8"))
+ query_context = command.call_args.args[0]
Review Comment:
IMO, the `test_client.get(....)` is designed for this test case rather than to use a mock function.
--
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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032728329
##########
superset/charts/data/api.py:
##########
@@ -130,6 +135,7 @@ def get_data(self, pk: int) -> Response:
"format", ChartDataResultFormat.JSON
)
json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL)
+ json_body["force"] = request.args.get("force", False)
Review Comment:
add `allow_none` argument in the rule.
```
force = fields.Boolean(
description="Should the queries be forced to load from the source. "
"Default: `false`",
allow_none=True
)
```
--
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] codecov[bot] commented on pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #22196:
URL: https://github.com/apache/superset/pull/22196#issuecomment-1323506132
# [Codecov](https://codecov.io/gh/apache/superset/pull/22196?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#22196](https://codecov.io/gh/apache/superset/pull/22196?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6ecf70e) into [master](https://codecov.io/gh/apache/superset/commit/6f2e76bc092a7c03cf3d0d47b7849ed124a75d54?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6f2e76b) will **decrease** coverage by `11.34%`.
> The diff coverage is `64.06%`.
> :exclamation: Current head 6ecf70e differs from pull request most recent head 9ee6398. Consider uploading reports for the commit 9ee6398 to get more accurate results
```diff
@@ Coverage Diff @@
## master #22196 +/- ##
===========================================
- Coverage 66.89% 55.55% -11.35%
===========================================
Files 1805 1835 +30
Lines 69071 69972 +901
Branches 7369 7590 +221
===========================================
- Hits 46208 38874 -7334
- Misses 20956 29132 +8176
- Partials 1907 1966 +59
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `52.51% <ø> (-0.32%)` | :arrow_down: |
| python | `57.46% <ø> (-23.99%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `50.90% <ø> (-0.17%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/22196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ackages/superset-ui-chart-controls/src/fixtures.ts](https://codecov.io/gh/apache/superset/pull/22196/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL2ZpeHR1cmVzLnRz) | `100.00% <ø> (ø)` | |
| [...d/packages/superset-ui-chart-controls/src/index.ts](https://codecov.io/gh/apache/superset/pull/22196/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL2luZGV4LnRz) | `100.00% <ø> (ø)` | |
| [...-chart-controls/src/sections/advancedAnalytics.tsx](https://codecov.io/gh/apache/superset/pull/22196/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NlY3Rpb25zL2FkdmFuY2VkQW5hbHl0aWNzLnRzeA==) | `14.28% <ø> (ø)` | |
| [...t-controls/src/sections/echartsTimeSeriesQuery.tsx](https://codecov.io/gh/apache/superset/pull/22196/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NlY3Rpb25zL2VjaGFydHNUaW1lU2VyaWVzUXVlcnkudHN4) | `33.33% <ø> (ø)` | |
| [...i-chart-controls/src/sections/forecastInterval.tsx](https://codecov.io/gh/apache/superset/pull/22196/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NlY3Rpb25zL2ZvcmVjYXN0SW50ZXJ2YWwudHN4) | `100.00% <ø> (ø)` | |
| [...perset-ui-chart-controls/src/sections/sections.tsx](https://codecov.io/gh/apache/superset/pull/22196/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NlY3Rpb25zL3NlY3Rpb25zLnRzeA==) | `71.42% <0.00%> (-16.08%)` | :arrow_down: |
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/22196/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `58.33% <ø> (ø)` | |
| [...d/packages/superset-ui-chart-controls/src/types.ts](https://codecov.io/gh/apache/superset/pull/22196/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3R5cGVzLnRz) | `100.00% <ø> (ø)` | |
| [.../packages/superset-ui-core/src/chart/types/Base.ts](https://codecov.io/gh/apache/superset/pull/22196/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvY2hhcnQvdHlwZXMvQmFzZS50cw==) | `100.00% <ø> (ø)` | |
| [...ntend/packages/superset-ui-core/src/color/index.ts](https://codecov.io/gh/apache/superset/pull/22196/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvY29sb3IvaW5kZXgudHM=) | `100.00% <ø> (ø)` | |
| ... and [569 more](https://codecov.io/gh/apache/superset/pull/22196/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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] mayurnewase commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032726570
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,59 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ # wrapping original class, so we can test output too.
+ with mock.patch(
+ "superset.charts.data.api.ChartDataCommand", wraps=ChartDataCommand
+ ) as command:
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+ data = json.loads(rv.data.decode("utf-8"))
+ query_context = command.call_args.args[0]
Review Comment:
so you suggesting, we just check the output and not the force param in query_context?
--
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] mayurnewase commented on pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on PR #22196:
URL: https://github.com/apache/superset/pull/22196#issuecomment-1327979931
Thanks for the reviews 😀
--
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] mayurnewase commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032731983
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,59 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ # wrapping original class, so we can test output too.
+ with mock.patch(
+ "superset.charts.data.api.ChartDataCommand", wraps=ChartDataCommand
+ ) as command:
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+ data = json.loads(rv.data.decode("utf-8"))
+ query_context = command.call_args.args[0]
Review Comment:
I wasn't sure if redis runs while integrations tests locally and on CI.
--
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] mayurnewase commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032727614
##########
superset/charts/data/api.py:
##########
@@ -130,6 +135,7 @@ def get_data(self, pk: int) -> Response:
"format", ChartDataResultFormat.JSON
)
json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL)
+ json_body["force"] = request.args.get("force", False)
Review Comment:
if we set force to be `None` then masrhmallow raises error saying it should be a boolean.
--
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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032731414
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,59 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ # wrapping original class, so we can test output too.
+ with mock.patch(
+ "superset.charts.data.api.ChartDataCommand", wraps=ChartDataCommand
+ ) as command:
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+ data = json.loads(rv.data.decode("utf-8"))
+ query_context = command.call_args.args[0]
Review Comment:
we should verify the `is_cached` field in the response.
```Python
test_client.post(f"api/v1/chart/{chart.id}/data/?force=true")
# should through cache
rv = test_client.post(f"api/v1/chart/{chart.id}/data/?force=true")
assert rv.json["result"][0]["is_cached"] is None
test_client.post(f"api/v1/chart/{chart.id}/data")
# should get response from the cache
rv = test_client.post(f"api/v1/chart/{chart.id}/data")
assert rv.json["result"][0]["is_cached"]
```
--
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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032731414
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,59 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ # wrapping original class, so we can test output too.
+ with mock.patch(
+ "superset.charts.data.api.ChartDataCommand", wraps=ChartDataCommand
+ ) as command:
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+ data = json.loads(rv.data.decode("utf-8"))
+ query_context = command.call_args.args[0]
Review Comment:
we should verify the `is_cached` field in the response.
```Python
test_client.post(f"api/v1/chart/{chart.id}/data/?force=true")
# should through cache
rv = test_client.post(f"api/v1/chart/{chart.id}/data/?force=true")
assert rv.json["result"][0]["is_cached"] is None
test_client.post(f"api/v1/chart/{chart.id}/data")
# should get response from the cache
rv = test_client.post(f"api/v1/chart/{chart.id}/data")
assert rv.json["result"][0]["is_cached"] is not None
```
--
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] mayurnewase commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1029960563
##########
superset/charts/data/api.py:
##########
@@ -130,6 +135,7 @@ def get_data(self, pk: int) -> Response:
"format", ChartDataResultFormat.JSON
)
json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL)
+ json_body["force"] = request.args.get("force", False)
Review Comment:
umm I think that is taken care by `_create_query_context_from_form` function below using mashmallow schema's load function.
I tested with false too and it's working as expected, do you still need this conversion 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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1029966009
##########
superset/charts/data/api.py:
##########
@@ -130,6 +135,7 @@ def get_data(self, pk: int) -> Response:
"format", ChartDataResultFormat.JSON
)
json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL)
+ json_body["force"] = request.args.get("force", False)
Review Comment:
if exists another deserialization helper on the json_body, the default value of `request.args.get("force")` should be `None` instead of `request.args.get("force", False)`, because the `request.args.get("force")` will always return `Optional[str]`.
--
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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1029966009
##########
superset/charts/data/api.py:
##########
@@ -130,6 +135,7 @@ def get_data(self, pk: int) -> Response:
"format", ChartDataResultFormat.JSON
)
json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL)
+ json_body["force"] = request.args.get("force", False)
Review Comment:
if exists another deserialization helper on the json_body, the default value of `request.args.get("force")` should be `None` instead of `request.args.get("force", False)`. The `request.args.get("force")` will always return `Optional[str]`.
--
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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032732475
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,59 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ # wrapping original class, so we can test output too.
+ with mock.patch(
+ "superset.charts.data.api.ChartDataCommand", wraps=ChartDataCommand
+ ) as command:
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+ data = json.loads(rv.data.decode("utf-8"))
+ query_context = command.call_args.args[0]
Review Comment:
yes, the [DATA_CACHE_CONFIG](https://github.com/apache/superset/blob/eba7b3d074350c3429778259baef19b7995f60ae/tests/integration_tests/superset_test_config.py#L106) has been set on the integrations test.
--
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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032733814
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,63 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data")
+
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+
+ data = json.loads(rv.data.decode("utf-8"))
+ assert rv.json["result"][0]["is_cached"] is None
+ assert rv.mimetype == "application/json"
+ assert data["result"][0]["status"] == "success"
+ assert data["result"][0]["rowcount"] == 2
Review Comment:
these codes are redundant as well. (same logic on the `test_chart_data_get`)
--
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] mayurnewase commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032736403
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,63 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data")
+
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+
+ data = json.loads(rv.data.decode("utf-8"))
Review Comment:
so instead of raw `test_client`, this file uses `get/post_assert_metric`, so just wanted to be consistent.
can we keep `get_assert_metric` ?
--
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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1029966009
##########
superset/charts/data/api.py:
##########
@@ -130,6 +135,7 @@ def get_data(self, pk: int) -> Response:
"format", ChartDataResultFormat.JSON
)
json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL)
+ json_body["force"] = request.args.get("force", False)
Review Comment:
if exists another deserialize helper on the json_body, the default value of `request.args.get("force")` should be `None` instead of `request.args.get("force", False)`. The `request.args.get("force")` will always return `Optional[str]`.
--
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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032731414
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,59 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ # wrapping original class, so we can test output too.
+ with mock.patch(
+ "superset.charts.data.api.ChartDataCommand", wraps=ChartDataCommand
+ ) as command:
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+ data = json.loads(rv.data.decode("utf-8"))
+ query_context = command.call_args.args[0]
Review Comment:
we should verify the `is_cached` field in the response.
```Python
test_client.post(f"api/v1/chart/{chart.id}/data/?force=true")
# should through cache
rv = test_client.post(f"api/v1/chart/{chart.id}/data/?force=true")
assert rv.json["result"][0]["is_cached"] is None
test_client.post(f"api/v1/chart/{chart.id}/data/")
# should get response from the cache
rv = test_client.post(f"api/v1/chart/{chart.id}/data/")
assert rv.json["result"][0]["is_cached"]
```
--
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] mayurnewase commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032731579
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,59 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ # wrapping original class, so we can test output too.
+ with mock.patch(
+ "superset.charts.data.api.ChartDataCommand", wraps=ChartDataCommand
+ ) as command:
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+ data = json.loads(rv.data.decode("utf-8"))
+ query_context = command.call_args.args[0]
Review Comment:
yeah good call, updating.
--
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] zhaoyongjie merged pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie merged PR #22196:
URL: https://github.com/apache/superset/pull/22196
--
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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032733841
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,63 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data")
+
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+
+ data = json.loads(rv.data.decode("utf-8"))
+ assert rv.json["result"][0]["is_cached"] is None
+ assert rv.mimetype == "application/json"
+ assert data["result"][0]["status"] == "success"
+ assert data["result"][0]["rowcount"] == 2
+
+ rv = self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data")
+ data = json.loads(rv.data.decode("utf-8"))
Review Comment:
and these
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,63 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data")
+
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+
+ data = json.loads(rv.data.decode("utf-8"))
+ assert rv.json["result"][0]["is_cached"] is None
+ assert rv.mimetype == "application/json"
+ assert data["result"][0]["status"] == "success"
+ assert data["result"][0]["rowcount"] == 2
+
+ rv = self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data")
+ data = json.loads(rv.data.decode("utf-8"))
+ assert rv.json["result"][0]["is_cached"] is not None
+ assert rv.mimetype == "application/json"
+ assert data["result"][0]["status"] == "success"
+ assert data["result"][0]["rowcount"] == 2
Review Comment:
these
--
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] mayurnewase commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032736403
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,63 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data")
+
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+
+ data = json.loads(rv.data.decode("utf-8"))
Review Comment:
so instead of raw `test_client`, this file uses `get/post_assert_metric`, so just wanted to be consistent.
--
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] mayurnewase commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032736429
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,63 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data")
+
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+
+ data = json.loads(rv.data.decode("utf-8"))
Review Comment:
can remove other data asserts.
--
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] mayurnewase commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032726570
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,59 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ # wrapping original class, so we can test output too.
+ with mock.patch(
+ "superset.charts.data.api.ChartDataCommand", wraps=ChartDataCommand
+ ) as command:
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+ data = json.loads(rv.data.decode("utf-8"))
+ query_context = command.call_args.args[0]
Review Comment:
so you suggesting, we just assert the output and not the force param in query_context?
--
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] mayurnewase commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032728267
##########
superset/charts/data/api.py:
##########
@@ -130,6 +135,7 @@ def get_data(self, pk: int) -> Response:
"format", ChartDataResultFormat.JSON
)
json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL)
+ json_body["force"] = request.args.get("force", False)
Review Comment:
so how about this, we set that param only if force is true?
--
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] mayurnewase commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032726613
##########
superset/charts/data/api.py:
##########
@@ -130,6 +135,7 @@ def get_data(self, pk: int) -> Response:
"format", ChartDataResultFormat.JSON
)
json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL)
+ json_body["force"] = request.args.get("force", False)
Review Comment:
ok updated.
--
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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032733814
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,63 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data")
+
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+
+ data = json.loads(rv.data.decode("utf-8"))
+ assert rv.json["result"][0]["is_cached"] is None
+ assert rv.mimetype == "application/json"
+ assert data["result"][0]["status"] == "success"
+ assert data["result"][0]["rowcount"] == 2
Review Comment:
these codes are redundant as well.
--
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] zhaoyongjie commented on a diff in pull request #22196: fix(reports): force data generation in csv reports
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22196:
URL: https://github.com/apache/superset/pull/22196#discussion_r1032733678
##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -862,6 +862,63 @@ def test_chart_data_get(self):
assert data["result"][0]["status"] == "success"
assert data["result"][0]["rowcount"] == 2
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_get_forced(self):
+ """
+ Chart data API: Test GET endpoint with force cache parameter
+ """
+ chart = db.session.query(Slice).filter_by(slice_name="Genders").one()
+ chart.query_context = json.dumps(
+ {
+ "datasource": {"id": chart.table.id, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+ "granularity": "ds",
+ "filters": [],
+ "extras": {
+ "having": "",
+ "having_druid": [],
+ "where": "",
+ },
+ "applied_time_extras": {},
+ "columns": ["gender"],
+ "metrics": ["sum__num"],
+ "orderby": [["sum__num", False]],
+ "annotation_layers": [],
+ "row_limit": 50000,
+ "timeseries_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ }
+ ],
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+
+ self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data")
+
+ rv = self.get_assert_metric(
+ f"api/v1/chart/{chart.id}/data/?force=true", "get_data"
+ )
+
+ data = json.loads(rv.data.decode("utf-8"))
Review Comment:
These codes are unrelated to this test case, should we remove these?
--
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