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 2021/01/09 03:02:26 UTC
[GitHub] [superset] ktmud opened a new issue #12375: [Explore] Saved metrics label overrides not working
ktmud opened a new issue #12375:
URL: https://github.com/apache/superset/issues/12375
#### Screenshots
<img src="https://user-images.githubusercontent.com/335541/104081231-2d55f980-51e2-11eb-96db-e6ee8003465a.png" width="600">
#### How to reproduce the bug
The new AdhocMetric control made it possible to add a custom label for saved metrics, but the added override doesn't actually affect anything. This is confusing. We should either disable "Click to edit label" or allow the label override to propagate to the selected pills and charts. The former is probably easier.
1. Add a saved metric
2. Click on the popover title to edit the metric label
### Environment
Latest master
### Checklist
Make sure to follow these steps before submitting your issue - thank you!
- [ ] I have checked the superset logs for python stacktraces and included it here as text if there are any.
- [x] I have reproduced the issue with at least the latest released version of superset.
- [ ] I have checked the issue tracker for the same issue and I haven't found one similar.
### Additional context
BTW, I really think saved metrics should be the first tab (at least when the dataset has some saved metrics), because the point of having saved metrics is to have an easy access to them. We should also encourage the use of saved metrics as it: 1) helps organizations reuse the same metric definition; 2) saves the labor of reconfigure the same 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.
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 edited a comment on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-757114387
I've heard many times at Airbnb that it's beneficial to have shared metric definitions. Other company may have different needs, but the discussion can start from some simple reasoning.
What was the reasoning behind putting SIMPLE as the default tab? Is it because we don't want to show users an empty list of saved metrics? Then what do you think of conditionally change the default tab for new metrics based on whether there are saved metrics in the datasource or not?
----------------------------------------------------------------
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.
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] villebro commented on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
villebro commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-757129875
@ktmud @junlincc I like the idea of supporting custom labels for saved metrics. If you agree I can fix 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.
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 edited a comment on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-757114387
I've heard many times at Airbnb that it's beneficial to have shared metric definitions. Other company may have different needs, but the discussion can start from some simple reasoning.
What was the reasoning behind putting SIMPLE as the default tab? Is it because we don't want to show users an empty list of saved metrics? Then what do you think of conditionally change the default tab for new metrics based on whether there are saved metrics in the datasource or not?
----------------------------------------------------------------
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.
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] junlincc removed a comment on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
junlincc removed a comment on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-757093691
cant reproduce if i understand correctly.
----------------------------------------------------------------
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.
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] junlincc commented on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
junlincc commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-757130144
> @ktmud @junlincc I like the idea of supporting custom labels for saved metrics. If you agree I can fix this.
please do! but hold off from switching tabs. @ktmud your request and reasoning makes sense, please allow research and design process to take place first.
----------------------------------------------------------------
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.
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 issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-757403712
> @ktmud @junlincc I like the idea of supporting custom labels for saved metrics. If you agree I can fix this.
@villebro could you hold until #10270 and https://github.com/apache-superset/superset-ui/pull/889 are merged? I just need to add more test cases and fix the CI. There are some (relatively large) refactoring related to QueryObject and formData, which may very likely conflict with whatever changes you will make in this area.
----------------------------------------------------------------
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.
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] junlincc commented on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
junlincc commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-757093691
----------------------------------------------------------------
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.
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] junlincc removed a comment on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
junlincc removed a comment on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-757093691
cant reproduce if i understand correctly.
----------------------------------------------------------------
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.
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] geido commented on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
geido commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-841795599
@ktmud The change is applied indeed. The problem you might be perceiving is that the change to the label is not applied on the fly. Check the video attached.
If we want to apply it on the fly, we might be discussing quite a big change as the results and the charts are built dynamically from the data received after the request.
Another possible solution is to force-run the query when the label name is changed and saved. I am not sure if that would be optimal though.
https://user-images.githubusercontent.com/60598000/118393357-719d4e80-b647-11eb-8e8e-d4001ab589de.mp4
--
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.
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 issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-843233789
What you had is a adhoc metric, not a saved 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.
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] villebro commented on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
villebro commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-757129875
@ktmud @junlincc I like the idea of supporting custom labels for saved metrics. If you agree I can fix 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.
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] junlincc commented on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
junlincc commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-843642419
https://github.com/apache/superset/issues/14595
--
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.
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] villebro commented on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
villebro commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-757420359
> @villebro could you hold until #10270 and https://github.com/apache-superset/superset-ui/pull/889 are merged? I just need to add more test cases and fix the CI. There are some (relatively large) refactoring related to QueryObject and formData, which may very likely conflict with whatever changes you will make in this area.
Sure thing 👍🏼
----------------------------------------------------------------
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.
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 issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-757114387
I've heard many times at Airbnb that it's beneficial to have shared metric definitions. Other company may have different needs, but the discussion can start from some simple reasoning.
What was the reasoning behind putting SIMPLE as the default tab? Is it because we don't want to show users an empty list of saved metrics? Then what do you think of conditionally change the default tab based on whether there are saved metrics in the datasource or not?
----------------------------------------------------------------
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.
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 issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-841559674
@geido The bug with the custom label not reflected in select metrics has been fixed. But unless I missed any followup PRs, this label still doesn't apply to the query results and visualizations of most 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.
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] geido commented on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
geido commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-841795599
@ktmud The change is applied indeed. The problem you might be perceiving is that the change to the label is not applied on the fly. Check the video attached.
If we want to apply it on the fly, we might be discussing quite a big change as the results and the charts are built dynamically from the data received after the request.
Another possible solution is to force-run the query when the label name is changed and saved. I am not sure if that would be optimal though.
https://user-images.githubusercontent.com/60598000/118393357-719d4e80-b647-11eb-8e8e-d4001ab589de.mp4
--
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.
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] geido commented on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
geido commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-840482068
@junlincc @ktmud can't reproduce this on master. The custom name of the metric is correctly shown. Is there any other place where the custom name should be propagated?
<img width="1052" alt="Screen Shot 2021-05-13 at 13 55 11" src="https://user-images.githubusercontent.com/60598000/118116386-e461bc00-b3f2-11eb-917d-d5f15f6b0489.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.
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 issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-757114387
----------------------------------------------------------------
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.
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 issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-848098923
As stated in the issue description:
> We should either disable "Click to edit label" or allow the label override to propagate to the selected pills and charts. The former is probably easier.
The bug was fixed by the former approach at some point. But I believe this thread was open because we also discussed actually allowing overriding saved metrics which is more work and, as you discovered, requires backend changes. Maybe we should have opened a new issue to track this instead of keep this open.
--
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.
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] junlincc commented on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
junlincc commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-757093890
> BTW, I really think saved metrics should be the first tab (at least when the dataset has some saved metrics), because the point of having saved metrics is to have an easy access to them. We should also encourage the use of saved metrics as it: 1) helps organizations reuse the same metric definition; 2) saves the labor of reconfigure the same metric.
can we conduct user research or at least a poll to have some data point to support this proposed change?
----------------------------------------------------------------
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.
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] junlincc commented on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
junlincc commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-757093691
cant reproduce if i understand correctly.
----------------------------------------------------------------
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.
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] geido commented on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
geido commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-844964512
Sorry for the confusion there, but the reason why I was looking at the 'adhoc metrics' is because the 'saved metrics' do not have the ability to change the title. This is explicitly turned off here https://github.com/apache/superset/blob/master/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx#L116.
I tried to force-enabling it but indeed changing the title does not affect anything, not even the label itself. This seems to be leaning toward a feature request more than a bug fix.
Additionally, as the labels for the charts and the query results are returned by the backend, the implementation will require backend support.
CC @junlincc @rusackas
--
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.
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] geido commented on issue #12375: [Explore] Saved metrics label overrides not working
Posted by GitBox <gi...@apache.org>.
geido commented on issue #12375:
URL: https://github.com/apache/superset/issues/12375#issuecomment-840477872
I am having a look at 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.
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