sijie commented on a change in pull request #9246:
URL: https://github.com/apache/pulsar/pull/9246#discussion_r561328245
##########
File path:
pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/InstanceUtils.java
##########
@@ -173,4 +176,25 @@ public static PulsarClient createPulsarClient(String
pulsarServiceUrl, Authentic
}
return null;
}
+
+ public static PulsarAdmin createPulsarAdminClient(String
pulsarWebServiceUrl, AuthenticationConfig authConfig) throws
PulsarClientException {
+ PulsarAdminBuilder pulsarAdminBuilder = null;
+ if (isNotBlank(pulsarWebServiceUrl)) {
+ pulsarAdminBuilder =
PulsarAdmin.builder().serviceHttpUrl(pulsarWebServiceUrl);
+ if (authConfig != null) {
+ if (isNotBlank(authConfig.getClientAuthenticationPlugin())
+ &&
isNotBlank(authConfig.getClientAuthenticationParameters())) {
+
pulsarAdminBuilder.authentication(authConfig.getClientAuthenticationPlugin(),
+ authConfig.getClientAuthenticationParameters());
+ }
+ if (isNotBlank(authConfig.getTlsTrustCertsFilePath())) {
+
pulsarAdminBuilder.tlsTrustCertsFilePath(authConfig.getTlsTrustCertsFilePath());
+ }
+
pulsarAdminBuilder.allowTlsInsecureConnection(authConfig.isTlsAllowInsecureConnection());
+
pulsarAdminBuilder.enableTlsHostnameVerification(authConfig.isTlsHostnameVerificationEnable());
+ }
+ return pulsarAdminBuilder.build();
+ }
+ return null;
Review comment:
since the method throws an exception, it might be worth throwing an
exception saying the `pulsarWebServiceUrl` is null.
##########
File path:
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdFunctions.java
##########
@@ -617,6 +617,8 @@ protected void validateFunctionConfigs(FunctionConfig
functionConfig) {
protected String DEPRECATED_brokerServiceUrl;
@Parameter(names = "--broker-service-url", description = "The URL for
Pulsar broker")
protected String brokerServiceUrl;
+ @Parameter(names = "--web-service-url", description = "The URL for
Pulsar web service")
+ protected String webServiceUrl;
Review comment:
I think we need to make this an optional field for backward
compatibility.
##########
File path: pulsar-functions/api-java/pom.xml
##########
@@ -49,6 +49,13 @@
<scope>compile</scope>
</dependency>
+ <dependency>
+ <groupId>${project.groupId}</groupId>
+ <artifactId>pulsar-client-admin-original</artifactId>
Review comment:
@freeznet Can you move the admin API to a separate module (i.e. move
them to a `pulsar-client-admin-api` module)? So we don't need to reference an
implementation module in the `api-java` module.
##########
File path:
pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/JavaInstanceStarter.java
##########
@@ -132,6 +132,9 @@
@Parameter(names = "--pending_async_requests", description = "Max pending
async requests per instance", required = false)
public int maxPendingAsyncRequests = 1000;
+ @Parameter(names = "--web_serviceurl", description = "Pulsar Web Service
Url", required = false)
+ public String webServiceUrl;
Review comment:
Make this field optional for backward compatibility.
##########
File path:
pulsar-functions/localrun/src/main/java/org/apache/pulsar/functions/LocalRunner.java
##########
@@ -135,6 +135,8 @@ public RuntimeEnv convert(String value) {
protected String stateStorageServiceUrl;
@Parameter(names = "--brokerServiceUrl", description = "The URL for the
Pulsar broker", hidden = true)
protected String brokerServiceUrl;
+ @Parameter(names = "--webServiceUrl", description = "The URL for the
Pulsar web service", hidden = true)
+ protected String webServiceUrl;
Review comment:
Make this optional for backward compatibility.
##########
File path:
pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java
##########
@@ -355,6 +357,14 @@
args.add("--pulsar_serviceurl");
args.add(pulsarServiceUrl);
+ if (instanceConfig.getFunctionDetails().getRuntime() ==
Function.FunctionDetails.Runtime.JAVA) {
+ // TODO: for now only Java function context exposed pulsar admin,
so python/go no need to pass this argument
+ // until pulsar admin client enabled in python/go function context.
+ if (pulsarWebServiceUrl != null) {
Review comment:
we need to add a flag in the function worker config to control whether
to expose PulsarAdmin through context. Because the new function worker will add
this `--web_serviceurl` to the command line that is not able to recognize by
the function instance. So we need to have a flag to allow people to do the
following update sequence:
1. Update function worker with exposing pulsar admin disabled.
2. Update all the functions to the new image that understand
`--web_serviceurl`
3. Update function worker to enable exposing pulsar admin.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
