You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/09/16 23:14:14 UTC
[GitHub] [pulsar] EronWright opened a new pull request #12072: [Issue 12040][web] Topic lookup with listener header
EronWright opened a new pull request #12072:
URL: https://github.com/apache/pulsar/pull/12072
Master Issue: #12040
### Motivation
This PR introduces an optional HTTP header `X-Pulsar-ListenerName` to the `TopicLookup` operation of the Pulsar Admin API. The header supplies the listener name to use when the broker has multiple advertised listeners. Today the `TopicLookup` operation accepts a query parameter `listenerName` for that same purpose.
The motivation for adding a header-based alternative is to improve support smart listener selection via HTTP gateways or proxies that are capable of rewriting headers (see: [Istio virtual services](https://istio.io/latest/docs/reference/config/networking/virtual-service/#Headers)).
See #12040 for more background.
### Modifications
- Modify `TopicLookup` operation to use a header (query string takes precedence).
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
This change is a trivial rework / code cleanup without any test coverage.
### 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: yes
- 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 and label this PR (if you have committer privilege).
Need to update docs?
- [ ] doc
(javadocs)
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] EronWright commented on pull request #12072: [PIP 95][Issue 12040][web] Topic lookup with listener header
Posted by GitBox <gi...@apache.org>.
EronWright commented on pull request #12072:
URL: https://github.com/apache/pulsar/pull/12072#issuecomment-941523076
This is ready to merge. The test failure seems unrelated:
```
Error: org.apache.pulsar.broker.admin.TopicPoliciesTest.testDisableSubscribeRate(org.apache.pulsar.broker.admin.TopicPoliciesTest)
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] EronWright commented on pull request #12072: [PIP 95][Issue 12040][web] Topic lookup with listener header
Posted by GitBox <gi...@apache.org>.
EronWright commented on pull request #12072:
URL: https://github.com/apache/pulsar/pull/12072#issuecomment-923175384
@codelipenghui since this is a part of PIP-95, should probably target 2.9 not 2.8.2.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] EronWright edited a comment on pull request #12072: [PIP 95][Issue 12040][web] Topic lookup with listener header
Posted by GitBox <gi...@apache.org>.
EronWright edited a comment on pull request #12072:
URL: https://github.com/apache/pulsar/pull/12072#issuecomment-923175384
@codelipenghui since this is a part of PIP-95 feature work, should probably target 2.9 not 2.8.2. Some of the other PRs are just bug fixes that could be in 2.8.2 in my opinion, but not this one.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] addisonj commented on pull request #12072: [PIP 95][Issue 12040][web] Topic lookup with listener header
Posted by GitBox <gi...@apache.org>.
addisonj commented on pull request #12072:
URL: https://github.com/apache/pulsar/pull/12072#issuecomment-941638303
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] EronWright commented on pull request #12072: [Issue 12040][web] Topic lookup with listener header
Posted by GitBox <gi...@apache.org>.
EronWright commented on pull request #12072:
URL: https://github.com/apache/pulsar/pull/12072#issuecomment-921903045
@wangjialing218 I agree, the advertised listener configuration does not apply to web service endpoints, it applies only to the Pulsar protocol endpoints. This PR is limited in scope. Let's discuss on the dev mailing list about whether #12040 should include this aspect.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] EronWright commented on pull request #12072: [PIP 95][Issue 12040][web] Topic lookup with listener header
Posted by GitBox <gi...@apache.org>.
EronWright commented on pull request #12072:
URL: https://github.com/apache/pulsar/pull/12072#issuecomment-941523076
This is ready to merge. The test failure seems unrelated:
```
Error: org.apache.pulsar.broker.admin.TopicPoliciesTest.testDisableSubscribeRate(org.apache.pulsar.broker.admin.TopicPoliciesTest)
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #12072: [PIP 95][Issue 12040][web] Topic lookup with listener header
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #12072:
URL: https://github.com/apache/pulsar/pull/12072#issuecomment-938449117
the failure is about a test related to this PR
>
> INFO]
> Error: Failures:
> Error: org.apache.pulsar.broker.lookup.http.v2.TopicLookupTest.testListenerName(org.apache.pulsar.broker.lookup.http.v2.TopicLookupTest)
> [INFO] Run 1: PASS
> Error: Run 2: TopicLookupTest.testListenerName:53->JerseyTest.target:593->JerseyTest.target:579 ยป NullPointer
> [INFO]
> [INFO]
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] addisonj commented on pull request #12072: [PIP 95][Issue 12040][web] Topic lookup with listener header
Posted by GitBox <gi...@apache.org>.
addisonj commented on pull request #12072:
URL: https://github.com/apache/pulsar/pull/12072#issuecomment-941638303
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui merged pull request #12072: [PIP 95][Issue 12040][web] Topic lookup with listener header
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #12072:
URL: https://github.com/apache/pulsar/pull/12072
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wangjialing218 commented on pull request #12072: [Issue 12040][web] Topic lookup with listener header
Posted by GitBox <gi...@apache.org>.
wangjialing218 commented on pull request #12072:
URL: https://github.com/apache/pulsar/pull/12072#issuecomment-921663472
I think it's not enough to only add the listener head to `lookupTopicAsync`.
For web request, there is some admin operation which may return a HTTP REDIRECT response with the http url address of the broker own the target topic. For example send a create subscription request to a broker which is not the owner of topic.
In this case, REDIRECT response using the internal webSeviceUrl and the client which send the HTTP requst may not able to connect to the REDIRECT address from outside.
Current there is only `brokerSeviceUrl` in advertised address, To support advertised listener for HTTP, we need add `webSeviceUrl` to advertised address first.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] EronWright commented on pull request #12072: [PIP 95][Issue 12040][web] Topic lookup with listener header
Posted by GitBox <gi...@apache.org>.
EronWright commented on pull request #12072:
URL: https://github.com/apache/pulsar/pull/12072#issuecomment-942768071
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org