leekeiabstraction commented on code in PR #22:
URL:
https://github.com/apache/flink-connector-prometheus/pull/22#discussion_r2042391665
##########
flink-connector-prometheus/src/main/java/org/apache/flink/connector/prometheus/sink/PrometheusSink.java:
##########
@@ -37,7 +37,7 @@
/** Sink implementation accepting {@link PrometheusTimeSeries} as inputs. */
@PublicEvolving
-public class PrometheusSink extends AsyncSinkBase<PrometheusTimeSeries,
Types.TimeSeries> {
+public class PrometheusSink<InputT> extends AsyncSinkBase<InputT,
Types.TimeSeries> {
Review Comment:
Also on name of type parameter, should we follow convention of using capital
letter?
##########
flink-connector-prometheus/src/main/java/org/apache/flink/connector/prometheus/sink/PrometheusSink.java:
##########
@@ -37,7 +37,7 @@
/** Sink implementation accepting {@link PrometheusTimeSeries} as inputs. */
@PublicEvolving
-public class PrometheusSink extends AsyncSinkBase<PrometheusTimeSeries,
Types.TimeSeries> {
+public class PrometheusSink<InputT> extends AsyncSinkBase<InputT,
Types.TimeSeries> {
Review Comment:
Is this a breaking change? Would current code which has reference to
PrometheusSink compile successfully?
##########
flink-connector-prometheus/src/main/java/org/apache/flink/connector/prometheus/sink/PrometheusTimeSeriesConverter.java:
##########
@@ -22,42 +22,23 @@
import org.apache.flink.connector.base.sink.writer.ElementConverter;
import org.apache.flink.connector.prometheus.sink.prometheus.Types;
+import java.util.UnknownFormatConversionException;
+
/**
* Converts the sink input {@link PrometheusTimeSeries} into the Protobuf
{@link Types.TimeSeries}
* that are sent to Prometheus.
*/
@PublicEvolving
-public class PrometheusTimeSeriesConverter
- implements ElementConverter<PrometheusTimeSeries, Types.TimeSeries> {
-
- private static final String METRIC_NAME_LABEL_NAME = "__name__";
+public class PrometheusTimeSeriesConverter<InputT>
+ implements ElementConverter<InputT, Types.TimeSeries> {
Review Comment:
Similar comment on breaking change.
##########
flink-connector-prometheus/src/main/java/org/apache/flink/connector/prometheus/sink/PrometheusSinkBuilder.java:
##########
@@ -30,9 +30,8 @@
/** Builder for Sink implementation. */
@PublicEvolving
-public class PrometheusSinkBuilder
- extends AsyncSinkBaseBuilder<
- PrometheusTimeSeries, Types.TimeSeries, PrometheusSinkBuilder>
{
+public class PrometheusSinkBuilder<InputT>
+ extends AsyncSinkBaseBuilder<InputT, Types.TimeSeries,
PrometheusSinkBuilder<InputT>> {
Review Comment:
Similar comment on breaking change and also type parameter.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
