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/05/10 17:20:07 UTC
[GitHub] [superset] eschutho opened a new pull request, #20014: add form_data with query
eschutho opened a new pull request, #20014:
URL: https://github.com/apache/superset/pull/20014
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a diff in pull request #20014: add form_data with query
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #20014:
URL: https://github.com/apache/superset/pull/20014#discussion_r870754117
##########
superset/charts/schemas.py:
##########
@@ -199,7 +199,7 @@ class ChartPostSchema(Schema):
datasource_id = fields.Integer(description=datasource_id_description, required=True)
datasource_type = fields.String(
description=datasource_type_description,
- validate=validate.OneOf(choices=("druid", "table", "view")),
+ validate=validate.OneOf(choices=("druid", "table", "view", "query")),
Review Comment:
can we change these to be the enums 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] hughhhh commented on a diff in pull request #20014: add form_data with query
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #20014:
URL: https://github.com/apache/superset/pull/20014#discussion_r870754416
##########
scripts/tests/run.sh:
##########
@@ -26,19 +26,20 @@ function reset_db() {
echo --------------------
echo Reseting test DB
echo --------------------
- docker-compose stop superset-tests-worker superset || true
- RESET_DB_CMD="psql \"postgresql://${DB_USER}:${DB_PASSWORD}@127.0.0.1:5432\" <<-EOF
+ # docker-compose stop superset-tests-worker superset || true
+ psql -U ${DB_USER}
Review Comment:
do we want to check in these changes?
--
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 #20014: add form_data with query
Posted by GitBox <gi...@apache.org>.
eschutho closed pull request #20014: add form_data with query
URL: https://github.com/apache/superset/pull/20014
--
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] hughhhh commented on a diff in pull request #20014: add form_data with query
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #20014:
URL: https://github.com/apache/superset/pull/20014#discussion_r870753068
##########
tests/integration_tests/explore/form_data/api_tests.py:
##########
@@ -89,6 +99,30 @@ def test_post(client, chart_id: int, dataset_id: int):
assert resp.status_code == 201
+@pytest.mark.usefixtures("get_query_datasource")
+def test_post_with_query(client, chart_id: int, query_id: int):
+ login(client, "admin")
+ form_data = {
+ "adhoc_filters": [],
+ "viz_type": "sankey",
+ "groupby": ["target"],
+ "metric": "sum__value",
+ "row_limit": 5000,
+ "slice_id": chart_id,
+ "time_range_endpoints": ["inclusive", "exclusive"],
+ "datasource": f"{query_id}__query",
+ }
+ payload = {
+ "datasource_id": query_id,
+ "chart_id": chart_id,
+ "form_data": json.dumps(form_data),
+ "datasource_type": "query",
+ }
+ resp = client.post("api/v1/explore/form_data", json=payload)
+ print(resp.data)
Review Comment:
remove this
--
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 #20014: add form_data with query
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20014:
URL: https://github.com/apache/superset/pull/20014#discussion_r877253526
##########
scripts/tests/run.sh:
##########
@@ -26,19 +26,20 @@ function reset_db() {
echo --------------------
echo Reseting test DB
echo --------------------
- docker-compose stop superset-tests-worker superset || true
- RESET_DB_CMD="psql \"postgresql://${DB_USER}:${DB_PASSWORD}@127.0.0.1:5432\" <<-EOF
+ # docker-compose stop superset-tests-worker superset || true
+ psql -U ${DB_USER}
Review Comment:
no, they're just for testing
--
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 #20014: add form_data with query
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20014:
URL: https://github.com/apache/superset/pull/20014#discussion_r869503329
##########
superset/explore/form_data/commands/state.py:
##########
@@ -24,3 +24,4 @@ class TemporaryExploreState(TypedDict):
dataset_id: int
chart_id: Optional[int]
form_data: str
+ datasource_type: str
Review Comment:
use DatasourceType enum
--
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 #20014: add form_data with query
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20014:
URL: https://github.com/apache/superset/pull/20014#discussion_r869503059
##########
superset/explore/form_data/commands/parameters.py:
##########
@@ -23,7 +23,8 @@
@dataclass
class CommandParameters:
actor: User
- dataset_id: int = 0
+ datasource_type: str
Review Comment:
use the DatasourceType enum
--
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 #20014: add form_data with query
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20014:
URL: https://github.com/apache/superset/pull/20014#discussion_r869501226
##########
superset/explore/form_data/commands/create.py:
##########
@@ -39,20 +39,23 @@ def __init__(self, cmd_params: CommandParameters):
def run(self) -> str:
self.validate()
try:
- dataset_id = self._cmd_params.dataset_id
+ datasource_id = self._cmd_params.datasource_id
+ datasource_type = self._cmd_params.datasource_type
chart_id = self._cmd_params.chart_id
tab_id = self._cmd_params.tab_id
actor = self._cmd_params.actor
form_data = self._cmd_params.form_data
- check_access(dataset_id, chart_id, actor)
- contextual_key = cache_key(session.get("_id"), tab_id, dataset_id, chart_id)
+ check_access(datasource_id, chart_id, actor, datasource_type)
+ contextual_key = cache_key(
+ session.get("_id"), tab_id, datasource_id, chart_id
Review Comment:
missing datasource_type 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