-
Notifications
You must be signed in to change notification settings - Fork 305
Description
Describe the bug
Currently, PropertyMapEventListener
implements only one method from PolarisEventListener
– all others are ignored. It is unclear if this is on purpose.
Furthermore:
- The API is not ideal:
transformAndSendEvent
takes aHashMap
parameter, and it is unclear whether the requirement for aHashMap
(instead ofMap
) is legit. - The only implemented method –
onAfterRefreshTable
does not serialize all the event fields. Again, it is unclear if this is by design or an oversight.
And finally, the whole point of having an abstract class that transforms events into Maps, only to have the Maps transformed into Json (cf. AwsCloudWatchEventListener
) seems unnecessary.
If the target serialization format is Json, it would be beneficial to provide Jackson serialization info in the event types themselves, e.g.:
public record AfterRefreshTableEvent(
@JsonProperty("catalog_name")
String catalogName,
@JsonProperty("table_identifier")
TableIdentifier tableIdentifier)
implements PolarisEvent {}
Or alternatively:
@JsonNaming(SnakeCaseStrategy.class)
public record AfterRefreshTableEvent(
String catalogName,
TableIdentifier tableIdentifier)
implements PolarisEvent {}
The above would be naturally serialized as:
{
"catalog_name" : "test_catalog",
"table_identifier" : {
"namespace" : [ "test_namespace" ],
"name" : "test_table"
}
}
Without the need for PropertyMapEventListener
.
PropertyMapEventListener
will also become problematic for larger payloads e.g. TableMetadata
as the serialization logic is not well-defined (which fields to include? which case? which order?)
To Reproduce
No response
Actual Behavior
No response
Expected Behavior
No response
Additional context
No response
System information
No response