Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed InvocationTargetException in pulsar-admin #1492

Merged
merged 3 commits into from Apr 4, 2018

Conversation

jai1
Copy link
Contributor

@jai1 jai1 commented Apr 3, 2018

Currently in master:

./bin/pulsar-admin 
class java.lang.reflect.InvocationTargetException: null

Reason is that in CmdFunctions.java:639 we expect the admin to be an instance of PulsarAdminWithFunctions while PulsarAdminBuilderImpl.build() builds an instance PulsarAdmin

I tried building PulsarAdminWithFunctions in PulsarAdminBuilderImpl instead of PulsarAdmin but ran into lots of errors so I just commented out adding pulsar functions to PulsarAdminTool, in order to get master working since I need to develop on Admin Tools.

I believe the problematic PR is 6230ab4 but can't say for sure since we use SNAPSHOT bookkeeper versions now.

@jai1 jai1 self-assigned this Apr 3, 2018
@sijie
Copy link
Member

sijie commented Apr 4, 2018

@jai1 I think it is #1327 broke the admin tool. so a better fix is to fix PulsarAdminTool to construct PulsarAdminWithFunctions.

@jai1
Copy link
Contributor Author

jai1 commented Apr 4, 2018

@sijie You are right #1327 broke the pulsar-admin

Even after breaking my head for 2 hours I couldn't figure out a way of resolving the cycling dependency b/w pulsar-functions, pulsar-admin and pulsar-client

@sijie
Copy link
Member

sijie commented Apr 4, 2018

Let me work on that

@jai1
Copy link
Contributor Author

jai1 commented Apr 4, 2018

retest this please

Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall looks good to me

@@ -71,7 +71,7 @@
private final PersistentTopics persistentTopics;
private final NonPersistentTopics nonPersistentTopics;
private final ResourceQuotas resourceQuotas;

private final ClientConfigurationData clientConfigData;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: better use spaces rather than tab

public PulsarAdmin(String serviceUrl, ClientConfigurationData pulsarConfig) throws PulsarClientException {
this.auth = pulsarConfig != null ? pulsarConfig.getAuthentication() : new AuthenticationDisabled();
public PulsarAdmin(String serviceUrl, ClientConfigurationData clientConfigData) throws PulsarClientException {
this.clientConfigData = clientConfigData;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: wrong indent

*/
public ClientConfigurationData getClientConfigData() {
return clientConfigData;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: spaces ?

@jai1
Copy link
Contributor Author

jai1 commented Apr 4, 2018

retest this please

@merlimat
Copy link
Contributor

merlimat commented Apr 4, 2018

retest this please

@sijie
Copy link
Member

sijie commented Apr 4, 2018

@jai1 it seems that CmdFunctionsTest is failing

@jai1 jai1 changed the title Removed Pulsar Functions from AdminTool Fixed InvocationTargetException in pulsar-admin Apr 4, 2018
@merlimat
Copy link
Contributor

merlimat commented Apr 4, 2018

replicator flaky test. retest this please

@jai1
Copy link
Contributor Author

jai1 commented Apr 4, 2018

retest this please

@jai1 jai1 merged commit 1edf4d9 into apache:master Apr 4, 2018
@jai1 jai1 deleted the adminClusterSet branch April 4, 2018 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants