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

Add setter for the reader subscription name #8801

Merged
merged 4 commits into from
Dec 7, 2020

Conversation

315157973
Copy link
Contributor

Fixes #8787

Motivation

In the current reader, we set the subscription name combine with a random part.

Modifications

add subscription setter

Verifying this change

ReaderTest#testReaderSubName

@315157973
Copy link
Contributor Author

/pulsarbot run-failure-checks

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Awesome work.
I left a few little comments about the test.
Can you please check?

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Looks great !

@sijie sijie added this to the 2.8.0 milestone Dec 4, 2020
@sijie sijie added area/client doc-required Your PR changes impact docs and you will update later. labels Dec 4, 2020
@315157973
Copy link
Contributor Author

/pulsarbot run-failure-checks

@jiazhai
Copy link
Member

jiazhai commented Dec 5, 2020

/pulsarbot run-failure-checks

@sijie sijie merged commit 8089f36 into apache:master Dec 7, 2020
RobertIndie pushed a commit to RobertIndie/pulsar that referenced this pull request Dec 8, 2020
Fixes apache#8787

### Motivation
In the current reader, we set the subscription name combine with a random part.

### Modifications
add subscription setter

### Verifying this change
ReaderTest#testReaderSubName
sijie pushed a commit that referenced this pull request Dec 8, 2020
Master Issue: #8787 

### Motivation

Currently, the reader subscription name can only be generated internally randomly in the C++ client.
Java client part is at #8801 

### Modifications

Add a setter for the reader's internal subscription name.

### Verifying this change

This change is already covered by existing tests, such as *testSubscriptionNameSetting*, *testSetSubscriptionNameAndPrefix* and *testMultiSameSubscriptionNameReaderShouldFail*.
zymap pushed a commit to zymap/pulsar that referenced this pull request Dec 9, 2020
…he#8823)

Master Issue: apache#8787 

### Motivation

Currently, the reader subscription name can only be generated internally randomly in the C++ client.
Java client part is at apache#8801 

### Modifications

Add a setter for the reader's internal subscription name.

### Verifying this change

This change is already covered by existing tests, such as *testSubscriptionNameSetting*, *testSetSubscriptionNameAndPrefix* and *testMultiSameSubscriptionNameReaderShouldFail*.
@315157973 315157973 deleted the reader-name branch December 9, 2020 09:15
codelipenghui pushed a commit that referenced this pull request Dec 21, 2020
Master Issue: #8787 

### Motivation

Currently, the reader subscription name can only be generated internally randomly in the C++ client.
Java client part is at #8801 

### Modifications

Add a setter for the reader's internal subscription name.

### Verifying this change

This change is already covered by existing tests, such as *testSubscriptionNameSetting*, *testSetSubscriptionNameAndPrefix* and *testMultiSameSubscriptionNameReaderShouldFail*.

(cherry picked from commit 408f9e6)
codelipenghui pushed a commit that referenced this pull request Dec 21, 2020
Master Issue: #8787 

### Motivation

Currently, the reader subscription name can only be generated internally randomly in the C++ client.
Java client part is at #8801 

### Modifications

Add a setter for the reader's internal subscription name.

### Verifying this change

This change is already covered by existing tests, such as *testSubscriptionNameSetting*, *testSetSubscriptionNameAndPrefix* and *testMultiSameSubscriptionNameReaderShouldFail*.

(cherry picked from commit 408f9e6)
@Anonymitaet Anonymitaet removed the doc-required Your PR changes impact docs and you will update later. label Apr 9, 2021
wolfstudy pushed a commit to apache/pulsar-client-go that referenced this pull request Mar 29, 2022
### Motivation
allow config reader's subscription name, follow java's feature apache/pulsar#8801

### Modifications
add param `SubscriptionName` in `ReaderOptions`

### Verifying this change
add the test for setting the subscritpion name
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.

Add setter for the reader subscription name
7 participants