shibd commented on code in PR #20902:
URL: https://github.com/apache/pulsar/pull/20902#discussion_r1296039274
##########
pulsar-io/aerospike/src/main/java/org/apache/pulsar/io/aerospike/AerospikeSinkConfig.java:
##########
@@ -38,21 +42,50 @@ public class AerospikeSinkConfig implements Serializable {
private String columnName;
// Optional
+ @FieldDoc(
+ required = false,
+ defaultValue = "",
+ sensitive = true,
+ help = "The username for authentication."
+ )
private String userName;
+ @FieldDoc(
+ required = false,
+ defaultValue = "",
+ sensitive = true,
+ help = "The password for authentication."
+ )
private String password;
private String keySet;
private int maxConcurrentRequests = 100;
private int timeoutMs = 100;
private int retries = 1;
+ /**
+ * @deprecated Use {@link #load(String, SinkContext)} instead.
+ */
+ @Deprecated
public static AerospikeSinkConfig load(String yamlFile) throws IOException
{
ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
return mapper.readValue(new File(yamlFile), AerospikeSinkConfig.class);
}
+ public static AerospikeSinkConfig load(String yamlFile, SinkContext
context) throws IOException {
+ ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
+ return load(mapper.readValue(new File(yamlFile), new
TypeReference<Map<String, Object>>() {}), context);
+ }
+
+ /**
+ * @deprecated Use {@link #load(Map, SinkContext)} instead.
+ */
+ @Deprecated
public static AerospikeSinkConfig load(Map<String, Object> map) throws
IOException {
ObjectMapper mapper = new ObjectMapper();
return mapper.readValue(mapper.writeValueAsString(map),
AerospikeSinkConfig.class);
}
+
+ public static AerospikeSinkConfig load(Map<String, Object> map,
SinkContext context) {
Review Comment:
> I am conflicted about removing those public methods. I agree this class is
cluttered, but it introduces an unnecessary breaking change to outright remove
them.
For each connector, no user to use its class, right? We just need to make
sure that the configuration is compatible.
I don't think removing these public methods would be a breaking change.
> What exactly is the connector framework's responsibility?
The io framework is responsible for loading the configuration from the YAML
and converting it to the Map. So for the connector, we only need cover to
deserialize the correct configuration from the Map.
##########
pulsar-io/aerospike/src/main/java/org/apache/pulsar/io/aerospike/AerospikeSinkConfig.java:
##########
@@ -38,21 +42,50 @@ public class AerospikeSinkConfig implements Serializable {
private String columnName;
// Optional
+ @FieldDoc(
+ required = false,
+ defaultValue = "",
+ sensitive = true,
+ help = "The username for authentication."
+ )
private String userName;
+ @FieldDoc(
+ required = false,
+ defaultValue = "",
+ sensitive = true,
+ help = "The password for authentication."
+ )
private String password;
private String keySet;
private int maxConcurrentRequests = 100;
private int timeoutMs = 100;
private int retries = 1;
+ /**
+ * @deprecated Use {@link #load(String, SinkContext)} instead.
+ */
+ @Deprecated
public static AerospikeSinkConfig load(String yamlFile) throws IOException
{
ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
return mapper.readValue(new File(yamlFile), AerospikeSinkConfig.class);
}
+ public static AerospikeSinkConfig load(String yamlFile, SinkContext
context) throws IOException {
+ ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
+ return load(mapper.readValue(new File(yamlFile), new
TypeReference<Map<String, Object>>() {}), context);
+ }
+
+ /**
+ * @deprecated Use {@link #load(Map, SinkContext)} instead.
+ */
+ @Deprecated
public static AerospikeSinkConfig load(Map<String, Object> map) throws
IOException {
ObjectMapper mapper = new ObjectMapper();
return mapper.readValue(mapper.writeValueAsString(map),
AerospikeSinkConfig.class);
}
+
+ public static AerospikeSinkConfig load(Map<String, Object> map,
SinkContext context) {
Review Comment:
> I am conflicted about removing those public methods. I agree this class is
cluttered, but it introduces an unnecessary breaking change to outright remove
them.
For each connector, no user to use its class, right? We just need to make
sure that the configuration is compatible.
I don't think removing these public methods would be a breaking change.
> What exactly is the connector framework's responsibility?
The io framework is responsible for loading the configuration from the YAML
and converting it to the Map. So for the connector, we only need cover to
deserialize the correct configuration from the Map.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
