rdblue commented on a change in pull request #31476:
URL: https://github.com/apache/spark/pull/31476#discussion_r570608230
##########
File path:
sql/catalyst/src/main/java/org/apache/spark/sql/connector/read/Scan.java
##########
@@ -102,4 +102,13 @@ default MicroBatchStream toMicroBatchStream(String
checkpointLocation) {
default ContinuousStream toContinuousStream(String checkpointLocation) {
throw new UnsupportedOperationException(description() + ": Continuous scan
are not supported");
}
+
+ /**
+ * Returns an array of supported custom metrics with name and description.
+ * By default it returns empty array.
+ */
+ default CustomMetric[] supportedCustomMetrics() {
Review comment:
I can see the argument for a separate trait, but I would opt not to add
this in one. We want to encourage sources to provide metrics. It's easier to
see that there's a method that can be implemented than to see an interface that
can be mixed in, especially when that interface might extend both `Scan` and
`Write` (and not extend from either). There's also just more cost with having a
lot of interfaces for very specific things.
I think an optional method is a good balance of concerns: it is optional,
but easily discovered and doesn't add another interface that an implementer
needs to know about and know which class to implement it 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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
