Skip to content

Driver NotificationFilter config #533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

thelonelyvulpes
Copy link
Contributor

@thelonelyvulpes thelonelyvulpes commented Oct 24, 2022

Add tests for:

  • Driver notification filter config. hello message bolt 5.2+
  • Session notification filter config. begin and run message on bolt 5.2+
  • severity & category being populated on notifications

@thelonelyvulpes thelonelyvulpes marked this pull request as ready for review October 24, 2022 16:14
@thelonelyvulpes thelonelyvulpes changed the title Notification filters. Driver NotificationFilter config Oct 24, 2022
Copy link
Contributor

@robsdedude robsdedude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works like a charm 🌟

The only thing that could be improved is documenting the notification structure. So maybe instead of having _reduce_notifications, we add another class like SummaryNotification to the protocol. See nutkit.protocol.responses.SummaryCounters for inspiration.

Copy link
Contributor

@bigmontz bigmontz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔔

@thelonelyvulpes thelonelyvulpes merged commit 4512d8a into neo4j-drivers:5.0 Mar 16, 2023
@thelonelyvulpes thelonelyvulpes deleted the feature/notifications branch March 16, 2023 17:36
Copy link
Contributor

@RichardIrons-neo4j RichardIrons-neo4j left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small thing about constants

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants