Fix notification config #254
Conversation
nsauk
force-pushed
the
fix-notification-config
branch
2 times, most recently
from
a7fe304 to
050b547
Compare
nsauk
force-pushed
the
fix-notification-config
branch
from
050b547 to
34863fa
Compare
| @@ -70,7 +70,7 @@ def notification_config(minimum_severity: nil, disabled_categories: nil) | |||
| org.neo4j.driver.internal.InternalNotificationConfig.new( | |||
| value_of(org.neo4j.driver.internal.InternalNotificationSeverity, minimum_severity), | |||
| disabled_categories | |||
| &.map { |value| value_of(org.neo4j.driver.internal.InternalNotificationCategory, value) } | |||
| &.map { |value| org.neo4j.driver.NotificationCategory.const_get(value.to_s.upcase) } | |||
There was a problem hiding this comment.
We have been dealing with this issue the following way: https://github.com/neo4jrb/neo4j-ruby-driver/blob/5.0/ruby/neo4j/driver/access_mode.rb
to have some level of stronger typing compatible with java.
I could be swayed over but could end up with complex converting code when the configuration is deeply nested resulting in runtime errors as users would be using nested symbol hashes instead of higher level types.
There was a problem hiding this comment.
Using NotificationClassification serves as well as a type of documentation.
|
Converted to draft because the spec is incorrect |
| org.neo4j.driver.internal.InternalNotificationConfig.new( | ||
| value_of(org.neo4j.driver.internal.InternalNotificationSeverity, minimum_severity), | ||
| disabled_categories | ||
| &.map { |value| value_of(org.neo4j.driver.internal.InternalNotificationCategory, value) } |
There was a problem hiding this comment.
@nsauk could you check. It appears to me that the only change needed was this line:
&.map { |value| value_of(org.neo4j.driver.NotificationClassification, value) }
There was a problem hiding this comment.
Maybe, additionally move the .or_else(nil) from value_of method to the line 71 as the value_of of java enum, which NotificationClassification now is, does not return Optional.
There was a problem hiding this comment.
I rewrote the rest of this method using the Java driver's public API so we rely less on internals.
InternalNotificationSeverity works with value_of, but NotificationSeverity fails because it's an interface. As far as I can understand, we can't use the same approach for both (unless it's const_get).
No description provided.