Skip to content

Conversation

@BewareMyPower
Copy link
Contributor

Fixes #9449

Motivation

This is a catchup work for #5716 that supports multiple topic subscriptions across multiple namespaces.

Modifications

  • Move the check for namespace in MultiTopicsConsumerImpl to PatternMultiTopicsConsumerImpl that uses a regex subscription.
  • Fix the existed tests for subscription on topics across different namespaces.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as BasicEndToEndTest.testMultiTopicsConsumerDifferentNamespace.

@BewareMyPower
Copy link
Contributor Author

And I've a question. Should we also add tests for Python client? Since it only affects Client::subscribe API and there're no related tests in Python tests before.

@BewareMyPower
Copy link
Contributor Author

/pulsarbot run-failure-checks

@BewareMyPower BewareMyPower changed the title [C++] Remove namespace check for MultiTopicsConsumerImpl [WIP][C++] Remove namespace check for MultiTopicsConsumerImpl Feb 7, 2021
@BewareMyPower
Copy link
Contributor Author

I found there's an existed Python test for multiple topics, I'll fix the test.

@BewareMyPower BewareMyPower changed the title [WIP][C++] Remove namespace check for MultiTopicsConsumerImpl [C++] Remove namespace check for MultiTopicsConsumerImpl Feb 7, 2021
@BewareMyPower
Copy link
Contributor Author

/pulsarbot run-failure-checks

1 similar comment
@BewareMyPower
Copy link
Contributor Author

/pulsarbot run-failure-checks

@sijie sijie merged commit 69e9322 into apache:master Feb 8, 2021
codelipenghui pushed a commit that referenced this pull request Feb 8, 2021
Fixes #9449 

### Motivation

This is a catchup work for #5716 that supports multiple topic subscriptions across multiple namespaces.

### Modifications

- Move the check for namespace in `MultiTopicsConsumerImpl` to `PatternMultiTopicsConsumerImpl` that uses a regex subscription.
- Fix the existed tests for subscription on topics across different namespaces.

### Verifying this change

This change is already covered by existing tests, such as *BasicEndToEndTest.testMultiTopicsConsumerDifferentNamespace*.

(cherry picked from commit 69e9322)
codelipenghui pushed a commit to streamnative/pulsar-archived that referenced this pull request Feb 8, 2021
Fixes apache#9449 

### Motivation

This is a catchup work for apache#5716 that supports multiple topic subscriptions across multiple namespaces.

### Modifications

- Move the check for namespace in `MultiTopicsConsumerImpl` to `PatternMultiTopicsConsumerImpl` that uses a regex subscription.
- Fix the existed tests for subscription on topics across different namespaces.

### Verifying this change

This change is already covered by existing tests, such as *BasicEndToEndTest.testMultiTopicsConsumerDifferentNamespace*.

(cherry picked from commit 69e9322)
(cherry picked from commit 3031550)
@codelipenghui codelipenghui added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Feb 18, 2021
codelipenghui pushed a commit that referenced this pull request Feb 18, 2021
Fixes #9449 

### Motivation

This is a catchup work for #5716 that supports multiple topic subscriptions across multiple namespaces.

### Modifications

- Move the check for namespace in `MultiTopicsConsumerImpl` to `PatternMultiTopicsConsumerImpl` that uses a regex subscription.
- Fix the existed tests for subscription on topics across different namespaces.

### Verifying this change

This change is already covered by existing tests, such as *BasicEndToEndTest.testMultiTopicsConsumerDifferentNamespace*.

(cherry picked from commit 69e9322)
@BewareMyPower BewareMyPower deleted the bewaremypower/cpp-subscribe-different-ns branch March 22, 2021 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to subscribe to different topics on different namespaces with Python client

4 participants