michaeljmarshall commented on code in PR #20902:
URL: https://github.com/apache/pulsar/pull/20902#discussion_r1296078918
##########
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:
> For each connector, no user to use its class, right? We just need to make
sure that the configuration is compatible.
I think you're right here. Part of the issue is likely the ambiguous
boundaries for users. It's probably possible to use this class as a dependency
in another project, but I doubt there is really an expectation that these
classes are meant to be used elsewhere. The primary risk would be for someone
slightly modifying a connector.
> 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.
As long as we assume these classes are for pulsar's internal use, I agree
that this is the framework's responsibility.
The last question is really whether we think this change is valuable.
@eolivelli is asking in another comment on this PR whether this is the right
design.
--
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]
