darenwkt commented on code in PR #22:
URL:
https://github.com/apache/flink-connector-prometheus/pull/22#discussion_r2054497015
##########
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?
Yes, this is a breaking change, current code will not compile successfully
as it is missing the InputType specification, I have updated the PR description
with a breaking change summary to explain this in more details
> Also on name of type parameter, should we follow convention of using
capital letter?
Thanks, this is a good shout, have renamed from "InputType" to "IN"
##########
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:
Addressed in PR description's breaking change summary
##########
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:
Addressed in PR description's breaking change summary
--
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
