Skip to content

Conversation

@vineeth1995
Copy link
Contributor

@vineeth1995 vineeth1995 commented Sep 27, 2022

Motivation

The pulsar-perf-tool should be able to take "proxyServiceUrl" and "proxyProtocol" as an input arguments as it can be provided by the user.

Modifications

Add "proxyServiceUrl" and "proxyProtocol" into PerformanceBaseArguments.java class.
Make changes into PerfClientUtils.java to pass "proxyServiceUrl" and "proxyProtocol" to clientBuilder.
Add unit test cases to verify

Verifying this change

This change added tests and can be verified as follows:

-Added unit test to verify functionality

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): ( no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

Check the box below or label this PR directly.

Need to update docs?

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

  • ready-to-test

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 27, 2022
@rdhabalia rdhabalia added this to the 2.11.0 milestone Sep 28, 2022
tlsTrustCertsFilePath=./path
tlsAllowInsecureConnection=true
tlsEnableHostnameVerification=true
proxyServiceURL=https://my-proxy-pulsar:4443/
Copy link
Contributor

Choose a reason for hiding this comment

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

this might fail any existing tests which are using this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The file is used by only one testcase "testReadFromConfigFile" which I have modified to work.

@rdhabalia rdhabalia merged commit d1b7c6a into apache:master Oct 4, 2022
@eolivelli
Copy link
Contributor

@rdhabalia @vineeth1995 one test fails like this:

2022-10-04T09:45:26.7691133Z stdout: Incorrect proxyProtocol name
2022-10-04T09:45:26.7691325Z
2022-10-04T09:45:26.7691613Z stderr: java.lang.IllegalArgumentException: No enum constant org.apache.pulsar.client.api.ProxyProtocol.
2022-10-04T09:45:26.7692069Z at java.base/java.lang.Enum.valueOf(Enum.java:273)
2022-10-04T09:45:26.7692493Z at org.apache.pulsar.client.api.ProxyProtocol.valueOf(ProxyProtocol.java:28)
2022-10-04T09:45:26.7693177Z at org.apache.pulsar.testclient.PerformanceBaseArguments.fillArgumentsFromProperties(PerformanceBaseArguments.java:153)
2022-10-04T09:45:26.7693864Z at org.apache.pulsar.testclient.PerformanceProducer.main(PerformanceProducer.java:310)

this is the run
https://github.com/apache/pulsar/actions/runs/3180185268/jobs/5185026501

@lhotari
Copy link
Member

lhotari commented Oct 4, 2022

@rdhabalia @vineeth1995 one test fails like this:

2022-10-04T09:45:26.7691133Z stdout: Incorrect proxyProtocol name
2022-10-04T09:45:26.7691325Z
2022-10-04T09:45:26.7691613Z stderr: java.lang.IllegalArgumentException: No enum constant org.apache.pulsar.client.api.ProxyProtocol.
2022-10-04T09:45:26.7692069Z at java.base/java.lang.Enum.valueOf(Enum.java:273)
2022-10-04T09:45:26.7692493Z at org.apache.pulsar.client.api.ProxyProtocol.valueOf(ProxyProtocol.java:28)
2022-10-04T09:45:26.7693177Z at org.apache.pulsar.testclient.PerformanceBaseArguments.fillArgumentsFromProperties(PerformanceBaseArguments.java:153)
2022-10-04T09:45:26.7693864Z at org.apache.pulsar.testclient.PerformanceProducer.main(PerformanceProducer.java:310)

this is the run https://github.com/apache/pulsar/actions/runs/3180185268/jobs/5185026501

fixed by #17930 . I also created #17932 as a follow-up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants