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 2020/07/05 08:06:46 UTC
[GitHub] [pulsar-client-go] nitishv opened a new pull request #305: [Issue 304][Reader] fixed panic in CreateReader API using custom MessageID for ReaderOptions
nitishv opened a new pull request #305:
URL: https://github.com/apache/pulsar-client-go/pull/305
Fixes #304
### Motivation
User of the client's client's [CreateReader](https://github.com/apache/pulsar-client-go/blob/master/pulsar/client.go#L109) API can use a custom type satisfying the [MessageID](https://github.com/apache/pulsar-client-go/blob/master/pulsar/message.go#L108) interface, when using it as a value for `StartMessageID` in [ReaderOptions](https://github.com/apache/pulsar-client-go/blob/master/pulsar/reader.go#L48) argument for the mentioned API.
The current reader creation does an untested type assertion here, when preparing the `consumerOptions` needed for creating a `partitionConsumer`.
https://github.com/apache/pulsar-client-go/blob/master/pulsar/reader_impl.go#L64
This assertion of `MessageID` as `*messageID` will fail unless an instance of `MessageID` is created from one of these exported APIs because `messageID` is unexported
https://github.com/apache/pulsar-client-go/blob/master/pulsar/message.go#L114-#L126
Note: `newMessageID` returns `*messageID` which satisfies `MessageID` interface as well.
### Modifications
Test the type assertion of `MessageID` as `*messageID`, if it fails, re-create a new `MessageID` using this
https://github.com/apache/pulsar-client-go/blob/975eb3781644ebe588fc142e53eadf39fe50341a/pulsar/impl_message.go#L97
This will ensure that the custom type can be re-created as a `*messageID` which can be used by `partitionConsumerOpts`
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
This change added tests and can be verified as follows:
Added unit test `TestReaderOnSpecificMessageWithCustomMessageID` in `github.com/apache/pulsar-client-go/pulsar` package.
Test uses a custom StartMessageID in ReaderOptions while creating a new reader to read from a specific message ID, rather than the pre-canned earliest/latest options.
CI script test result snippet below.
```bash
-- Ready to start tests
+ go test -race -coverprofile=/tmp/coverage -timeout=20m ./...
? github.com/apache/pulsar-client-go/examples/consumer [no test files]
? github.com/apache/pulsar-client-go/examples/consumer-listener [no test files]
? github.com/apache/pulsar-client-go/examples/producer [no test files]
? github.com/apache/pulsar-client-go/examples/reader [no test files]
ok github.com/apache/pulsar-client-go/integration-tests 1.732s coverage: 0.0% of statements
? github.com/apache/pulsar-client-go/perf [no test files]
ok github.com/apache/pulsar-client-go/pulsar 141.199s coverage: 81.2% of statements
ok github.com/apache/pulsar-client-go/pulsar/internal 1.658s coverage: 30.7% of statements
ok github.com/apache/pulsar-client-go/pulsar/internal/auth 1.028s coverage: 31.3% of statements
ok github.com/apache/pulsar-client-go/pulsar/internal/compression 1.041s coverage: 55.7% of statements
? github.com/apache/pulsar-client-go/pulsar/internal/pulsar_proto [no test files]
+ go tool cover -html=/tmp/coverage -o coverage.html
```
### Does this pull request potentially affect one of the following parts:
- 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
### Documentation
- Does this pull request introduce a new feature? : No
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] nitishv commented on pull request #305: [Issue 304][Reader] fixed panic in CreateReader API using custom MessageID for ReaderOptions
Posted by GitBox <gi...@apache.org>.
nitishv commented on pull request #305:
URL: https://github.com/apache/pulsar-client-go/pull/305#issuecomment-653856249
@wolfstudy @sijie @ghatage please have a look
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] Ghatage commented on pull request #305: [Issue 304][Reader] fixed panic in CreateReader API using custom MessageID for ReaderOptions
Posted by GitBox <gi...@apache.org>.
Ghatage commented on pull request #305:
URL: https://github.com/apache/pulsar-client-go/pull/305#issuecomment-654032363
Great stuff @nitishv! Excited to have you involved in the Pulsar community!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] wolfstudy merged pull request #305: [Issue 304][Reader] fixed panic in CreateReader API using custom MessageID for ReaderOptions
Posted by GitBox <gi...@apache.org>.
wolfstudy merged pull request #305:
URL: https://github.com/apache/pulsar-client-go/pull/305
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org