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/07/11 18:19:24 UTC
[GitHub] [superset] codyml opened a new pull request, #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
codyml opened a new pull request, #20673:
URL: https://github.com/apache/superset/pull/20673
<!---
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 -->
It looks like a URL redirect wasn't updated when Explore was moved to SPA, and as a result Selenium can't access the chart standalone mode. This PR updated the URL redirect.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
Check that Selenium URLs for charts in the form of `/superset/slice/32/?standalone=true` now redirect to `/explore/?form_data=%7B%22slice_id%22%3A%2032%7D&standalone=true` instead of `/superset/explore/?form_data=%7B%22slice_id%22%3A%2032%7D&standalone=True`.
### 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 #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #20673:
URL: https://github.com/apache/superset/pull/20673#discussion_r922182725
##########
superset/views/core.py:
##########
@@ -427,13 +427,13 @@ def slice(self, slice_id: int) -> FlaskResponse: # pylint: disable=no-self-use
_, slc = get_form_data(slice_id, use_slice_data=True)
if not slc:
abort(404)
- endpoint = "/superset/explore/?form_data={}".format(
+ endpoint = "/explore/?form_data={}".format(
parse.quote(json.dumps({"slice_id": slice_id}))
)
is_standalone_mode = ReservedUrlParameters.is_standalone_mode()
if is_standalone_mode:
- endpoint += f"&{ReservedUrlParameters.STANDALONE}={is_standalone_mode}"
+ endpoint += f"&{ReservedUrlParameters.STANDALONE}=true"
Review Comment:
the boolean type can be implicitly converted to string.
```shell
Python 3.8.6 (default, Nov 11 2020, 23:52:15)
[Clang 12.0.0 (clang-1200.0.32.21)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> str(True)
'True'
>>> str(False)
'False'
>>>
```
--
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] michael-s-molina merged pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
michael-s-molina merged PR #20673:
URL: https://github.com/apache/superset/pull/20673
--
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 #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1180912467
# [Codecov](https://codecov.io/gh/apache/superset/pull/20673?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 [#20673](https://codecov.io/gh/apache/superset/pull/20673?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3003312) into [master](https://codecov.io/gh/apache/superset/commit/ae306d6d1f76d6437e640c649e6a4c4e8b3060cb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ae306d6) will **decrease** coverage by `11.83%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #20673 +/- ##
===========================================
- Coverage 66.85% 55.02% -11.84%
===========================================
Files 1753 1753
Lines 65823 65823
Branches 7007 7007
===========================================
- Hits 44005 36218 -7787
- Misses 20032 27819 +7787
Partials 1786 1786
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `53.73% <0.00%> (ø)` | |
| python | `58.34% <0.00%> (-24.64%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `50.96% <0.00%> (ø)` | |
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/20673?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/reports/commands/execute.py](https://codecov.io/gh/apache/superset/pull/20673/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-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9leGVjdXRlLnB5) | `24.45% <ø> (-67.16%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/20673/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-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `34.74% <0.00%> (-42.62%)` | :arrow_down: |
| [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/20673/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/20673/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.14%)` | :arrow_down: |
| [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/20673/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%> (-88.89%)` | :arrow_down: |
| [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/20673/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-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `0.00% <0.00%> (-85.30%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/20673/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-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/key\_value/commands/delete\_expired.py](https://codecov.io/gh/apache/superset/pull/20673/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-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZV9leHBpcmVkLnB5) | `0.00% <0.00%> (-80.77%)` | :arrow_down: |
| [superset/dashboards/commands/importers/v0.py](https://codecov.io/gh/apache/superset/pull/20673/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9jb21tYW5kcy9pbXBvcnRlcnMvdjAucHk=) | `15.62% <0.00%> (-76.25%)` | :arrow_down: |
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/20673/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-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `25.30% <0.00%> (-68.68%)` | :arrow_down: |
| ... and [278 more](https://codecov.io/gh/apache/superset/pull/20673/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/20673?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/20673?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 [ae306d6...3003312](https://codecov.io/gh/apache/superset/pull/20673?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] michael-s-molina commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1189330533
> @michael-s-molina i am ok with merge this pr first if you think deck is low priority, and we probably need to cherry this in for release candidate.
@jinghua-qa Let's merge this first and follow up with a fix to deck charts.
--
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] codyml closed pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
codyml closed pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
URL: https://github.com/apache/superset/pull/20673
--
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] codyml commented on a diff in pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
codyml commented on code in PR #20673:
URL: https://github.com/apache/superset/pull/20673#discussion_r922283704
##########
superset/views/core.py:
##########
@@ -427,13 +427,13 @@ def slice(self, slice_id: int) -> FlaskResponse: # pylint: disable=no-self-use
_, slc = get_form_data(slice_id, use_slice_data=True)
if not slc:
abort(404)
- endpoint = "/superset/explore/?form_data={}".format(
+ endpoint = "/explore/?form_data={}".format(
parse.quote(json.dumps({"slice_id": slice_id}))
)
is_standalone_mode = ReservedUrlParameters.is_standalone_mode()
if is_standalone_mode:
- endpoint += f"&{ReservedUrlParameters.STANDALONE}={is_standalone_mode}"
+ endpoint += f"&{ReservedUrlParameters.STANDALONE}=true"
Review Comment:
For this one the implicit conversion to `True` was part of why the URL wasn't working: the frontend expected it to be lowercase, so I wanted to make that explicit. But, I did update the frontend so if others use implicit conversion from Python `bool` it should work!
--
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] github-actions[bot] commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1189359899
Ephemeral environment shutdown and build artifacts deleted.
--
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] sadpandajoe commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
sadpandajoe commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1189582306
🏷️ preset:2022.29
--
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] michael-s-molina commented on a diff in pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #20673:
URL: https://github.com/apache/superset/pull/20673#discussion_r922077985
##########
superset/views/redirects.py:
##########
@@ -34,25 +34,40 @@ class R(BaseSupersetView): # pylint: disable=invalid-name
"""used for short urls"""
@staticmethod
- def _validate_url(url: Optional[str] = None) -> bool:
- if url and (
- url.startswith("//superset/dashboard/")
- or url.startswith("//superset/explore/")
- ):
- return True
- return False
+ def _validate_explore_url(url: str) -> Optional[str]:
+ if url.startswith("//superset/explore/p/"):
+ return url
+
+ if url.startswith("//superset/explore"):
+ return "/" + url[10:] # Remove /superset from old Explore URLs
+
+ if url.startswith("//explore"):
+ return url
+
+ return None
+
+ @staticmethod
+ def _validate_dashboard_url(url: str) -> Optional[str]:
+ if url.startswith("//superset/explore/"):
Review Comment:
```suggestion
if url.startswith("//superset/dashboard/"):
```
--
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] github-actions[bot] commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1180937661
@rusackas Container image not yet published for this PR. Please try again when build is complete.
--
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] michael-s-molina commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1185460225
@codyml The license error was fixed [here](https://github.com/apache/superset/pull/20713) so you just need to rebase your PR.
--
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 pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1182875816
Hi @codyml, I look into the related codebase. I think you just change one line to fix it.
```
(superset) yongjie.zhao@:incubator-superset$ git diff superset/views/core.py
diff --git a/superset/views/core.py b/superset/views/core.py
index 298535011..8aedf8321 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -427,7 +427,7 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
_, slc = get_form_data(slice_id, use_slice_data=True)
if not slc:
abort(404)
- endpoint = "/superset/explore/?form_data={}".format(
+ endpoint = "/explore/?form_data={}".format(
parse.quote(json.dumps({"slice_id": slice_id}))
)
```
--
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] github-actions[bot] commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1180989802
@rusackas Ephemeral environment creation failed. Please check the Actions logs for details.
--
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 #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #20673:
URL: https://github.com/apache/superset/pull/20673#discussion_r918294158
##########
tests/integration_tests/core_tests.py:
##########
@@ -117,15 +117,6 @@ def test_dashboard_endpoint(self):
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_slice_endpoint(self):
self.login(username="admin")
- slc = self.get_slice("Girls", db.session)
- resp = self.get_resp("/superset/slice/{}/".format(slc.id))
- assert "Original value" in resp
- assert "List Roles" in resp
-
- # Testing overrides
- resp = self.get_resp("/superset/slice/{}/?standalone=true".format(slc.id))
- assert '<div class="navbar' not in resp
Review Comment:
if this is false now, what's the current value? Do we still have a standalone url without the nav?
--
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] rusackas commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
rusackas commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1180936116
/testenv up
--
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] rusackas commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
rusackas commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1180988365
/testenv up
--
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] codyml commented on a diff in pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
codyml commented on code in PR #20673:
URL: https://github.com/apache/superset/pull/20673#discussion_r922337474
##########
superset/views/redirects.py:
##########
@@ -34,25 +34,40 @@ class R(BaseSupersetView): # pylint: disable=invalid-name
"""used for short urls"""
@staticmethod
- def _validate_url(url: Optional[str] = None) -> bool:
- if url and (
- url.startswith("//superset/dashboard/")
- or url.startswith("//superset/explore/")
- ):
- return True
- return False
+ def _validate_explore_url(url: str) -> Optional[str]:
+ if url.startswith("//superset/explore/p/"):
+ return url
+
+ if url.startswith("//superset/explore"):
+ return "/" + url[10:] # Remove /superset from old Explore URLs
+
+ if url.startswith("//explore"):
+ return url
+
+ return None
+
+ @staticmethod
+ def _validate_dashboard_url(url: str) -> Optional[str]:
+ if url.startswith("//superset/explore/"):
Review Comment:
Dumb mistake thanks!
--
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] jinghua-qa commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
jinghua-qa commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1188063115
/testenv up FEATURE_THUMBNAILS=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] ktmud commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
ktmud commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1180873171
Should we make the url parameter case insensitive where it is consumed?
--
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] github-actions[bot] commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1180989773
@rusackas Container image not yet published for this PR. Please try again when build is complete.
--
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] codyml commented on a diff in pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
codyml commented on code in PR #20673:
URL: https://github.com/apache/superset/pull/20673#discussion_r918308290
##########
tests/integration_tests/core_tests.py:
##########
@@ -117,15 +117,6 @@ def test_dashboard_endpoint(self):
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_slice_endpoint(self):
self.login(username="admin")
- slc = self.get_slice("Girls", db.session)
- resp = self.get_resp("/superset/slice/{}/".format(slc.id))
- assert "Original value" in resp
- assert "List Roles" in resp
-
- # Testing overrides
- resp = self.get_resp("/superset/slice/{}/?standalone=true".format(slc.id))
- assert '<div class="navbar' not in resp
Review Comment:
I believe that now that Explore is in the SPA, the response doc just contains the SPA wrapper. I was assuming that `get_resp` doesn't download/execute the JS bundle, so I think these tests were only still passing by accident:
- `"Original value"` and `"List Roles"` are contained in the embedded bootstrap data but I don't think that bootstrap data is used by Explore anymore (it uses API endpoints now)
- `<div class="navbar'` isn't present in either standalone or non-standalone mode anymore
So, it seemed like these tests don't test anything meaningful anymore, with the exception of the 404 test below. I looked around for examples of backend tests for Dashboard, but didn't see much and assumed they had also been removed when Dashboard was brought into the SPA.
--
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 #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
ktmud commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1180972022
In general, restrictions on multi-choice or boolean inputs on the client side should be more relaxed unless it has security implications or significantly increases code complexity. This helps with usability---"you know my intention, just give me what I want already."
--
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] codyml commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
codyml commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1180945820
> Should we make the url parameter case insensitive where it is consumed?
@ktmud I think I'd lean slightly towards keeping it case-sensitive on the client side, just to keep the server-client contract as explicit as possible, but no strong feelings either way!
--
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 pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1185570181
I have tested it in my local. the command `superset compute-thumbnails -c` works and the thumbnail was generated.
<img width="1336" alt="image" src="https://user-images.githubusercontent.com/2016594/179237214-2fdd04b4-3a04-4065-8cf1-88b68bb71871.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] jinghua-qa commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
jinghua-qa commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1189238501
Verified Chart/Dashboard png for Alert and Report is working fine, thumbnail is working for Homepage, Chart and Dashboard
Found 1 issue that deck charts thumbnails are not showing
<img width="1715" alt="Screen Shot 2022-07-18 at 10 43 08 AM" src="https://user-images.githubusercontent.com/81597121/179796108-ae61bf31-3625-4ab8-a8a4-c34d22cdb40a.png">
<img width="1792" alt="Screen Shot 2022-07-18 at 10 42 26 AM" src="https://user-images.githubusercontent.com/81597121/179796122-a0160658-8950-4721-85c2-6f879f0a2846.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] github-actions[bot] commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1180937696
@rusackas Ephemeral environment creation failed. Please check the Actions logs for details.
--
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] michael-s-molina commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1183217948
@codyml Can we rebase this and check if there's any `/superset/explore` reference left?
--
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] kgabryje commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
kgabryje commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1185443746
Thanks for the fix! The code looks great to me, but I can't fully test it, because for some reason the reports don't work for me on localhost (unrelated to this PR)
--
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] github-actions[bot] commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1188068554
@jinghua-qa Ephemeral environment spinning up at http://34.222.120.222:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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] jinghua-qa commented on pull request #20673: fix(explore): Fix chart standalone URL for report/thumbnail generation
Posted by GitBox <gi...@apache.org>.
jinghua-qa commented on PR #20673:
URL: https://github.com/apache/superset/pull/20673#issuecomment-1189276965
@michael-s-molina i am ok with merge this pr first if you think deck is low priority, and we probably need to cherry this in for release candidate.
--
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