You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "mattitoo (via GitHub)" <gi...@apache.org> on 2023/06/02 09:30:20 UTC
[GitHub] [superset] mattitoo opened a new issue, #24271: [SIP] Proposal for adding a notification feature
mattitoo opened a new issue, #24271:
URL: https://github.com/apache/superset/issues/24271
**Motivation**
As maintainer of a Superset Instance, I want to be able to message my users, so I can inform them about possible downtimes, data troubles or other causes.
This can of course be achieved in various channels (Mail, Slack, Teams, etc) but it also makes sense to do this in the actual tool used, in this case right inside Superset.
**Proposed Change**
Luckily, Superset already has a messaging capability built in: so-called Toasts, which are displayed after certain actions like saving, refreshing dashboards and so on.
We have created a “management interface” for such toasts, in which new messages (we call them notifications) can be configured:
<img width="1561" alt="Bildschirmfoto 2023-05-30 um 15 37 25" src="https://github.com/apache/superset/assets/104143980/973c9869-0076-4feb-88b6-ffac2f63bb42">
<img width="833" alt="Bildschirmfoto 2023-05-30 um 15 37 39" src="https://github.com/apache/superset/assets/104143980/fd141bed-8b3a-4148-bd73-4fcf501bdd76">
Following parameters can be set:
· Active
· Name or Summary
· Category (currently 3 categories, these manage the color of the notification: Info=blue, Warning=yellow, Alert=red)
· Message: The message to be displayed.
· Time Range: The range in which the notification should be displayed
· Use daily timeframe: If active, the notification is only shown when the current time is inside the start and end time
· Re-trigger: Minimum of 5 Minutes to reduce the load on the database due to querying for new messages for each user
· Duration: How long until the notification disappears (it can always be closed via the ‘x’)
· Roles: Which roles should see that notification
For each user, the internal database is queried at login and at a certain interval to see if she/he has new notifications pending.
Example notification in action:
<img width="311" alt="Bildschirmfoto 2023-05-30 um 15 57 20" src="https://github.com/apache/superset/assets/104143980/3583f943-21f3-4b36-8947-f1adf3c1fb0d">
NOTE: This should be behind a feature flag like “ENABLE_UI_NOTIFICATIONS”
**New or Changed Public Interfaces**
Extends internal database to include two additional schemas: public.notification and public.notification.roles
<img width="392" alt="Bildschirmfoto 2023-05-30 um 15 49 42" src="https://github.com/apache/superset/assets/104143980/bf7f52a5-e4d6-4e4d-8f4c-b3b3b0b00773">
Extends API to include endpoints for management of Notifications
<img width="1151" alt="Bildschirmfoto 2023-05-30 um 15 51 10" src="https://github.com/apache/superset/assets/104143980/47d24754-aa9d-43df-bc54-1626f664c1a5">
Adds an entry in the Settings menu
<img width="316" alt="Bildschirmfoto 2023-05-30 um 15 56 22" src="https://github.com/apache/superset/assets/104143980/c4a80d8d-b803-49e6-98a9-5aa846395d41">
Adds new permission for notification management
<img width="1150" alt="Bildschirmfoto 2023-05-30 um 15 57 09" src="https://github.com/apache/superset/assets/104143980/1365f7c1-6d3e-4920-8bd9-1d300b1ac707">
**Migration Plan and Compatibility**
Database migration necessary to include new tables
**Rejected Alternatives**
Using another method of displaying notifications, e.g. as a banner on top of Superset or below the menu bar. Rejected in favor of using existing and proven methods already built in.
--
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.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] villebro commented on issue #24271: [SIP-96] Proposal for adding a notification feature
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1590939716
I agree, let's not overload the initial work, but let's make sure we keep these in mind early on so the initial design can accommodate iterative improvements later. To summarize; let's leave as much room for extensibility as possible.
--
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] villebro commented on issue #24271: [SIP] Proposal for adding a notification feature
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1573545101
I LOVE this proposal! I routinely need something similar, but hadn't even thought of having something like this. +1 for the proposal and an even bigger +1 for the well thought out UI + API.
Some thoughts:
- I know we aren't super consistent in always applying or not applying training slashes on REST endpoints, but I believe it's commonly accepted that leaving out the trailing slash is best practice. So maybe replace `/api/v1/notification/` with `/api/v1/notification` etc. @dpgaspar any thoughts here?
- The "Manage" menu item is maybe unnecessarily ambiguous. I've been thinking about starting a SIP to introduce a similar UI for cache warmup jobs, and if we do end up adding more scheduling functionality, maybe we could consider breaking them out under a new menu "Scheduled tasks". Maybe not now, but perhaps for Superset 4.0
- Instead of toasts, maybe important notifications should be displayed more prominently, e.g. on the subnav (the one with "Home" in the pic below), with a button to explicitly ack that the message has been read? Typical cases for such notifications could be scheduled downtimes, upgrade notifications etc. Also being able to add links to the messages would be a nice addition.
<img width="1189" alt="image" src="https://github.com/apache/superset/assets/33317356/a2bb6ab2-f81d-4d7d-a02e-05d96396fb81">
--
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 issue #24271: [SIP-96] Proposal for adding a notification feature
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1581248463
Thanks for the SIP! Numbering this as SIP-96. Please make sure to submit it on a [DISCUSS] thread on the Superset @dev mailing list, so we can start progressing it toward a vote. 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
Re: [I] [SIP-96] Proposal for adding a notification feature [superset]
Posted by "soniagtm (via GitHub)" <gi...@apache.org>.
soniagtm commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1982928782
Hi @mattitoo,
I wanted to inquire if there have been any updates regarding this proposal or its implementation, as it's a feature we're eagerly anticipating.
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] pankajsoni22 commented on issue #24271: [SIP] Proposal for adding a notification feature
Posted by "pankajsoni22 (via GitHub)" <gi...@apache.org>.
pankajsoni22 commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1573860992
I agree with the thought process, it's definitely a good step make this tool better and more useful.
--
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
Re: [I] [SIP-96] Proposal for adding a notification feature [superset]
Posted by "mattitoo (via GitHub)" <gi...@apache.org>.
mattitoo commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1746271043
Sorry, we have been bogged down with other issues.
Next up for us is switching to 3.0.1, where we will also make sure that this functionality works.
I will start the DISCUSS thread shortly.
--
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 issue #24271: [SIP-96] Proposal for adding a notification feature
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1591054890
> I agree, let's not overload the initial work, but let's make sure we keep these in mind early on so the initial design can accommodate iterative improvements later. To summarize; let's leave as much room for extensibility as possible.
@mattitoo @villebro Exactly. The important part is that the foundation considers these future points of development. As we can see by the comments, the proposed feature seems to have great potential. Thanks again for the proposal @mattitoo and for driving its development. I'll gladly help reviewing the work and maybe contributing 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
Re: [I] [SIP-96] Proposal for adding a notification feature [superset]
Posted by "mattitoo (via GitHub)" <gi...@apache.org>.
mattitoo commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1982946112
I am sorry there has not been much progress, we have been busy with other features, but will get started to make it "contributable" soon
--
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 issue #24271: [SIP-96] Proposal for adding a notification feature
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1587801184
Thanks for the SIP! Some thoughts:
- It would be a good idea to get @kasiazjc's input on the screens.
- I really like @villebro's idea of different types of notifications. A banner would be helpful for critical messages that need to persist while an event is happening.
- Regarding the HTML sanitization, you can always extend the default schema with `HTML_SANITIZATION_OVERRIDES` and keep it safe with a well defined Content Security Policy (CSP). The important part here is that notifications should respect these settings.
--
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] mattitoo commented on issue #24271: [SIP-96] Proposal for adding a notification feature
Posted by "mattitoo (via GitHub)" <gi...@apache.org>.
mattitoo commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1590725273
Very good points. I like the JINJA templating idea to automate messaging more.
Regarding snoozing and acknowledging: This is also a great idea. We were thinking about it, but at first abandoned it to get a working solution out of the way and not increase initial complexity too much. But once we have the PRs out, we should be thinking about this, i feel this is a very good thing to have.
--
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] mattitoo commented on issue #24271: [SIP] Proposal for adding a notification feature
Posted by "mattitoo (via GitHub)" <gi...@apache.org>.
mattitoo commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1573445441
We will also be implementing a "Preview" button for the notifications in the configuration overlay.
--
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] villebro commented on issue #24271: [SIP-96] Proposal for adding a notification feature
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1590706266
I agree with @michael-s-molina 's proposal to use `HTML_SANITIZATION` and `HTML_SANITIZATION_SCHEMA_EXTENSIONS` (I believe that's what they're called?). I feel this needs to be part of the initial PR, as it's already an established pattern in the codebase. Regarding toasts vs banners, I'm fine with @mattitoo 's proposal to keep the scope limited to toasts for now. Let's just make sure the initial body of work can stand on its own (=so the project isn't stuck with more dead wood if development interest simmers) and we don't paint ourselves into a corner in a way which makes it difficult to extend notifications beyond toasts. And to distribute the work, I'm quite happy to pick up the task of creating banner notifications after the initial feature is ready if nobody else is volunteering for it.
One more thing that came to mind - should we support Jinja templating in these notifications? This will make it possible to make dynamic messages, e.g. "You only have 2 days left to migrate X to Y" instead of "Please migrate X to Y by Z". This may also be something worth considering for the other fields, like Category: It's an `Info` when there's more than 7 days left, becomes and `Alert` at 3 days and `Danger` when there's less than 2 days. Also, I brought this up in my initial message, but I think we should also consider adding support for snoozing and acknowledging messages.
--
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] mattitoo commented on issue #24271: [SIP-96] Proposal for adding a notification feature
Posted by "mattitoo (via GitHub)" <gi...@apache.org>.
mattitoo commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1590645620
How we could continue on this:
- We need to do some refactoring of the existing code to make it nicer and more accessible
- While doing that, we would also check the HTML sanitization
- We create a PR to have the code checked etc
- After merging, whoever wants can work on expanding the functionality (like different types of notifications)
The reasoning is that the topic is already big in my opinion and maybe we should add additional complexity step by step.
Of course, I am open to other way of going forward as well, but the first 3 points are pretty clear ;)
--
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] mattitoo commented on issue #24271: [SIP] Proposal for adding a notification feature
Posted by "mattitoo (via GitHub)" <gi...@apache.org>.
mattitoo commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1573668795
Talking about links i realize i forgot to mention the elephant in the room: We explicitly disabled HTML sanitization for these toasts to be able to include links (and styling).
The reason behind this is that in a toast you only have limit space and you e.g. want to link to Release notes or similar. I understand this can be seen as a security risk, but for us this is not a big deal.
So, in our current implementation, HTML is possible within a notification.
As for other forms of notifications: That was my initial idea (to use banners or similar), but we went with the built-in way of using toasts because it was easier and within the known User Experience. We can still think about enhancing the functionality to include other notification types.
--
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
Re: [I] [SIP-96] Proposal for adding a notification feature [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on issue #24271:
URL: https://github.com/apache/superset/issues/24271#issuecomment-1745687796
Hi @mattitoo (and everyone) - I just wanted to check in on the status here. If we intend to move forward with this effort, someone will need to start a "DISCUSS" thread on the Apache Superset dev mailing list. After that, we can carry it forward to a vote. Ping me on Slack if help is needed.
--
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