aromanenko-dev commented on code in PR #22028:
URL: https://github.com/apache/beam/pull/22028#discussion_r906429881


##########
sdks/java/io/pulsar/src/main/java/org/apache/beam/sdk/io/pulsar/PulsarSourceDescriptor.java:
##########
@@ -50,14 +50,25 @@ public abstract class PulsarSourceDescriptor implements 
Serializable {
   @SchemaFieldName("admin_url")
   abstract String getAdminUrl();
 
+  @SchemaFieldName("authPluginClassName")
+  @Nullable
+  abstract String getAuthPluginClassName();
+
+  @SchemaFieldName("authParams")
+  @Nullable
+  abstract String getAuthParams();
+
   public static PulsarSourceDescriptor of(
       String topic,
       Long startOffsetTimestamp,
       Long endOffsetTimestamp,
       MessageId endMessageId,
       String clientUrl,
-      String adminUrl) {
+      String adminUrl,

Review Comment:
   Since this is a public method then it would be better to provide another an 
overloaded method with the same name and additional arguments. 
   
   If this class and method are not supposed to be used by user then it should 
be private or package private to avoid potential breaking changes like this.



##########
sdks/java/io/pulsar/src/test/java/org/apache/beam/sdk/io/pulsar/ReadFromPulsarDoFnTest.java:
##########
@@ -77,7 +77,7 @@ public void testInitialRestrictionWhenHasStartOffset() throws 
Exception {
     OffsetRange result =
         dofnInstance.getInitialRestriction(
             PulsarSourceDescriptor.of(
-                TOPIC, expectedStartOffset, null, null, SERVICE_URL, 
ADMIN_URL));
+                TOPIC, expectedStartOffset, null, null, SERVICE_URL, 
ADMIN_URL, null, null));

Review Comment:
   Please, add the tests with non-null auth values.



-- 
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]

Reply via email to