Introduce Notifications Filter - #26
Conversation
| The filter can be configured trough the optional `notificationsFilter`. This is an object which can contain the following properties: | ||
|
|
||
| * `minimumSeverityLevel` (optional) - Defines the minimum severity level . Possible values: `WARNING``, `INFORMATION` and `OFF`. | ||
| * `disabledCategories` (optional) - Defines the list of categories disabled to be returned. Possible values on the list: `HINT`, `UNRECOGNIZED`, `UNSUPPORTED`, `PERFORMANCE`, `TOPOLOGY`, `SECURITY`, `DEPRECATION`, `GENERIC` and `SCHEMA` |
There was a problem hiding this comment.
Maybe not worth to be documented, but I'm curios: will unknown categories be ignored or are supported values versioned (once we introduce the concept of API versions)?
There was a problem hiding this comment.
On the filter, the new categories will be supported as they are implemented in the driver, so I will need to document it in the version number and a introduced at on the new category.
On the result, the current behaviour is that unknown categories are omitted of the result. The notification arrives, but without category field.
There was a problem hiding this comment.
On the filter
Makes sense to me 👍
On the result, the current behaviour is that unknown categories are omitted of the result.
That sounds like it's worth documenting (i.e., that the category may be missing from the response). But taking a step back: why is that? The driver also exposes a string version of the category. The Query API could easily be using that value and with that have one less place to worry about the compatibility story.
There was a problem hiding this comment.
The Query API is returning it from the enum version on the Notification, not from the string.
There was a problem hiding this comment.
I'm echoing @robsdedude thoughts here, but besides creating issues with compatibility and API versions, this creates a fracture between server and API, which should feel one thing to users. The server shouldn't even be an entity for users: there's neo4j, and they can interact with it in a bunch of different ways. The fact that the server emits information that the API strips separates server and API in a way that I find detrimental to user experience.
There was a problem hiding this comment.
I've got it, but I don't think it worth to create a breaking change on behaviour on notifications since it will be deprecated in favor of GQLStatusObjects. Another point is that when Query API is running in the same process as the database (what we have right now), the moments where the category will be omitted because it's not implemented in the driver is quite short, at max one minor version if nothing really out of normal happen.
There was a problem hiding this comment.
The Query API is returning it from the enum version on the Notification, not from the string.
I understood that much. But that's not a good reason in itself, that's just the technical detail. It answers "how was it designed?", but not really "why was it designed like that?".
I don't think it worth to create a breaking change on behaviour on notifications
It wouldn't be a breaking change to switch to using the string would it? Unless the query API changes the capitalization of the categories. If it doesn't that switching to strings isn't a breaking change. It will a) never omit the value—that's fine, client code has to be able to accept responses with the field present any way b) in a future version propagate new categories instead of omitting them—which the next-next version with the new driver understanding the new category would do anyway.
it will be deprecated in favor of GQLStatusObjects
Fair enough, I guess. We can just sit it out (even though I assume it'd be a 10 LOC PR or so). But please take note somewhere not to repeat the same design mistake (at least I think it was a mistake).
|
Thanks for the documentation updates. The preview documentation has now been torn down - reopening this PR will republish it. |
| The filter can be configured trough the optional `notificationsFilter`. This is an object which can contain the following properties: | ||
|
|
||
| * `minimumSeverityLevel` (optional) - Defines the minimum severity level . Possible values: `WARNING``, `INFORMATION` and `OFF`. | ||
| * `disabledCategories` (optional) - Defines the list of categories disabled to be returned. Possible values on the list: `HINT`, `UNRECOGNIZED`, `UNSUPPORTED`, `PERFORMANCE`, `TOPOLOGY`, `SECURITY`, `DEPRECATION`, `GENERIC` and `SCHEMA` |
There was a problem hiding this comment.
I'm echoing @robsdedude thoughts here, but besides creating issues with compatibility and API versions, this creates a fracture between server and API, which should feel one thing to users. The server shouldn't even be an entity for users: there's neo4j, and they can interact with it in a bunch of different ways. The fact that the server emits information that the API strips separates server and API in a way that I find detrimental to user experience.
4 participants
2026.08