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/05 22:05:03 UTC
[GitHub] [superset] ktmud opened a new pull request, #19966: test: make tabbed dashboard a little more complex
ktmud opened a new pull request, #19966:
URL: https://github.com/apache/superset/pull/19966
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
Update the test dashboard "Tabbed dashboard" to have more nested tabs so we can test more complex use cases.
Also updated the `update_slice_ids` function to use dict instead of sorted lists so that slices in position_json can be matched more correctly.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
#### Before
<img width="987" alt="Xnip2022-05-05_15-02-12" src="https://user-images.githubusercontent.com/335541/167033047-a0836bf4-c109-42e5-9c44-407ff4d8453e.png">
#### After
<img width="982" alt="Xnip2022-05-05_14-36-59" src="https://user-images.githubusercontent.com/335541/167033043-c0f3f968-4ff5-4d89-8529-0fd26205bb00.png">
### 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] john-bodley commented on a diff in pull request #19966: test: make tabbed dashboard a little more complex
Posted by GitBox <gi...@apache.org>.
john-bodley commented on code in PR #19966:
URL: https://github.com/apache/superset/pull/19966#discussion_r872862839
##########
superset/cli/examples.py:
##########
@@ -54,8 +54,7 @@ def load_examples_run(
if load_test_data:
print("Loading [Tabbed dashboard]")
examples.load_tabbed_dashboard(only_metadata)
-
- if not load_test_data:
Review Comment:
Ha!
--
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] ktmud commented on a diff in pull request #19966: test: make tabbed dashboard a little more complex
Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #19966:
URL: https://github.com/apache/superset/pull/19966#discussion_r866348518
##########
superset/charts/schemas.py:
##########
@@ -156,7 +156,6 @@ class ChartEntityResponseSchema(Schema):
slice_name = fields.String(description=slice_name_description)
cache_timeout = fields.Integer(description=cache_timeout_description)
changed_on = fields.String(description=changed_on_description)
- modified = fields.String()
Review Comment:
By catch, this field returns invalid strings:
<img width="808" alt="Xnip2022-05-05_14-16-49" src="https://user-images.githubusercontent.com/335541/167033095-efae0202-3721-4918-b66a-726eb1177d74.png">
--
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] ktmud commented on pull request #19966: test: make tabbed dashboard a little more complex
Posted by GitBox <gi...@apache.org>.
ktmud commented on PR #19966:
URL: https://github.com/apache/superset/pull/19966#issuecomment-1124216233
@john-bodley I added an integration test for `superset load_examples` command. Can you take a look?
--
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] ktmud merged pull request #19966: test: make tabbed dashboard a little more complex
Posted by GitBox <gi...@apache.org>.
ktmud merged PR #19966:
URL: https://github.com/apache/superset/pull/19966
--
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] ktmud commented on a diff in pull request #19966: test: make tabbed dashboard a little more complex
Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #19966:
URL: https://github.com/apache/superset/pull/19966#discussion_r867270714
##########
superset/examples/world_bank.py:
##########
@@ -460,12 +459,6 @@ def create_slices(tbl: BaseDatasource) -> List[Slice]:
"meta": {"chartId": 49, "height": 50, "sliceName": "Treemap", "width": 8},
"type": "CHART",
},
- "CHART-3nc0d8sk": {
- "children": [],
- "id": "CHART-3nc0d8sk",
- "meta": {"chartId": 50, "height": 50, "sliceName": "Treemap", "width": 8},
- "type": "CHART",
- },
Review Comment:
Bycatch: this is a duplicate slice where the chart component isn't really used in the layout.
--
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] ktmud commented on a diff in pull request #19966: test: make tabbed dashboard a little more complex
Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #19966:
URL: https://github.com/apache/superset/pull/19966#discussion_r868868632
##########
superset/examples/misc_dashboard.py:
##########
@@ -211,11 +210,7 @@ def load_misc_dashboard() -> None:
"""
)
pos = json.loads(js)
- slices = (
- db.session.query(Slice).filter(Slice.slice_name.in_(misc_dash_slices)).all()
- )
- slices = sorted(slices, key=lambda x: x.id)
- update_slice_ids(pos, slices)
+ pos, slices = update_slice_ids(pos)
Review Comment:
Ah, right! I'm surprised test cases didn't catch 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] ktmud commented on a diff in pull request #19966: test: make tabbed dashboard a little more complex
Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #19966:
URL: https://github.com/apache/superset/pull/19966#discussion_r868869148
##########
superset/models/dashboard.py:
##########
@@ -218,7 +220,7 @@ def filter_sets_lst(self) -> Dict[int, FilterSet]:
}
@property
- def charts(self) -> List[BaseDatasource]:
+ def charts(self) -> List[str]:
Review Comment:
It was caught when I updated the file! This must be a very old piece of code.
--
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 #19966: test: make tabbed dashboard a little more complex
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #19966:
URL: https://github.com/apache/superset/pull/19966#issuecomment-1119090834
# [Codecov](https://codecov.io/gh/apache/superset/pull/19966?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 [#19966](https://codecov.io/gh/apache/superset/pull/19966?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (395813b) into [master](https://codecov.io/gh/apache/superset/commit/54bfd8375aa34bae928ca32bc0942d75ed7057fc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (54bfd83) will **decrease** coverage by `12.09%`.
> The diff coverage is `66.66%`.
> :exclamation: Current head 395813b differs from pull request most recent head cffe0c6. Consider uploading reports for the commit cffe0c6 to get more accurate results
```diff
@@ Coverage Diff @@
## master #19966 +/- ##
===========================================
- Coverage 66.28% 54.18% -12.10%
===========================================
Files 1712 1712
Lines 63961 63953 -8
Branches 6731 6731
===========================================
- Hits 42395 34652 -7743
- Misses 19855 27590 +7735
Partials 1711 1711
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `53.52% <66.66%> (+0.01%)` | :arrow_up: |
| python | `57.33% <66.66%> (-25.15%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `48.64% <0.00%> (+<0.01%)` | :arrow_up: |
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/19966?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/charts/schemas.py](https://codecov.io/gh/apache/superset/pull/19966/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-c3VwZXJzZXQvY2hhcnRzL3NjaGVtYXMucHk=) | `99.35% <ø> (-0.01%)` | :arrow_down: |
| [superset/examples/deck.py](https://codecov.io/gh/apache/superset/pull/19966/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-c3VwZXJzZXQvZXhhbXBsZXMvZGVjay5weQ==) | `0.00% <0.00%> (ø)` | |
| [superset/examples/misc\_dashboard.py](https://codecov.io/gh/apache/superset/pull/19966/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-c3VwZXJzZXQvZXhhbXBsZXMvbWlzY19kYXNoYm9hcmQucHk=) | `0.00% <0.00%> (ø)` | |
| [superset/examples/tabbed\_dashboard.py](https://codecov.io/gh/apache/superset/pull/19966/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-c3VwZXJzZXQvZXhhbXBsZXMvdGFiYmVkX2Rhc2hib2FyZC5weQ==) | `0.00% <0.00%> (ø)` | |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/superset/pull/19966/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-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `0.00% <0.00%> (-30.67%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/superset/pull/19966/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-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `71.29% <100.00%> (ø)` | |
| [superset/examples/helpers.py](https://codecov.io/gh/apache/superset/pull/19966/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-c3VwZXJzZXQvZXhhbXBsZXMvaGVscGVycy5weQ==) | `80.95% <100.00%> (+0.95%)` | :arrow_up: |
| [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/19966/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-c3VwZXJzZXQvdXRpbHMvZGFzaGJvYXJkX2ltcG9ydF9leHBvcnQucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset/key\_value/commands/upsert.py](https://codecov.io/gh/apache/superset/pull/19966/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-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3Vwc2VydC5weQ==) | `0.00% <0.00%> (-89.59%)` | :arrow_down: |
| [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/19966/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-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `0.00% <0.00%> (-89.37%)` | :arrow_down: |
| ... and [273 more](https://codecov.io/gh/apache/superset/pull/19966/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) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/19966?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/19966?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [54bfd83...cffe0c6](https://codecov.io/gh/apache/superset/pull/19966?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] john-bodley commented on a diff in pull request #19966: test: make tabbed dashboard a little more complex
Posted by GitBox <gi...@apache.org>.
john-bodley commented on code in PR #19966:
URL: https://github.com/apache/superset/pull/19966#discussion_r868858167
##########
superset/examples/misc_dashboard.py:
##########
@@ -211,11 +210,7 @@ def load_misc_dashboard() -> None:
"""
)
pos = json.loads(js)
- slices = (
- db.session.query(Slice).filter(Slice.slice_name.in_(misc_dash_slices)).all()
- )
- slices = sorted(slices, key=lambda x: x.id)
- update_slice_ids(pos, slices)
+ pos, slices = update_slice_ids(pos)
Review Comment:
Shouldn’t this be `slices = …`?
##########
superset/examples/helpers.py:
##########
@@ -39,17 +39,25 @@ def get_examples_folder() -> str:
return os.path.join(app.config["BASE_DIR"], "examples")
-def update_slice_ids(layout_dict: Dict[Any, Any], slices: List[Slice]) -> None:
- charts = [
+def update_slice_ids(pos: Dict[Any, Any]) -> List[Slice]:
Review Comment:
Why not return a set if order isn’t relevant?
##########
superset/models/dashboard.py:
##########
@@ -218,7 +220,7 @@ def filter_sets_lst(self) -> Dict[int, FilterSet]:
}
@property
- def charts(self) -> List[BaseDatasource]:
+ def charts(self) -> List[str]:
Review Comment:
How/why wasn’t this caught by Mypy?
--
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] ktmud commented on a diff in pull request #19966: test: make tabbed dashboard a little more complex
Posted by GitBox <gi...@apache.org>.
ktmud commented on code in PR #19966:
URL: https://github.com/apache/superset/pull/19966#discussion_r868867643
##########
superset/examples/helpers.py:
##########
@@ -39,17 +39,25 @@ def get_examples_folder() -> str:
return os.path.join(app.config["BASE_DIR"], "examples")
-def update_slice_ids(layout_dict: Dict[Any, Any], slices: List[Slice]) -> None:
- charts = [
+def update_slice_ids(pos: Dict[Any, Any]) -> List[Slice]:
Review Comment:
The type wouldn't match with an SQLA n:n relationship property which expects a list.
--
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