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 2022/12/02 16:45:23 UTC
[GitHub] [pulsar-client-go] flowchartsman opened a new pull request, #898: [Issue #897]
flowchartsman opened a new pull request, #898:
URL: https://github.com/apache/pulsar-client-go/pull/898
Fixes #897
Am not certain if this is the right place or behavior, so opening it up for comments. It might be that it is fine to panic on a `Flush()` to a non-batching producer, provided it's explicitly documented, but probably not. It's worth noting that it worked in the past, which is how I ran into the issue in the first place, since I left an errant `Flush()` call behind in some tests after disabling batching on a producer.
Master Issue: #<xyz>
### Motivation
No one wants a panic, especially one that is undocumented.
### Modifications
nil check.
### Verifying this change
- [X] Make sure that the change passes the CI checks.
I've included tests.
### Documentation
Since this issue would normally cause a panic, it's probably not necessary to document anything special, especially considering that a similar check is made elsewhere throughout the code. If requested, I can add a comment to the `Flush()` Godoc mentioning that it is a no-op for non-batching producers.
--
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-client-go] flowchartsman commented on pull request #898: [Issue #897] Flush on unbatched producer panics
Posted by "flowchartsman (via GitHub)" <gi...@apache.org>.
flowchartsman commented on PR #898:
URL: https://github.com/apache/pulsar-client-go/pull/898#issuecomment-1664625707
This issue was duplicated by #1064 (I guess no one searches issues any more) and fixed by #1065. Closing this as superceded.
--
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-client-go] flowchartsman closed pull request #898: [Issue #897] Flush on unbatched producer panics
Posted by "flowchartsman (via GitHub)" <gi...@apache.org>.
flowchartsman closed pull request #898: [Issue #897] Flush on unbatched producer panics
URL: https://github.com/apache/pulsar-client-go/pull/898
--
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-client-go] dimovnike commented on pull request #898: [Issue #897] Flush on unbatched producer panics
Posted by "dimovnike (via GitHub)" <gi...@apache.org>.
dimovnike commented on PR #898:
URL: https://github.com/apache/pulsar-client-go/pull/898#issuecomment-1663670422
> it might be that it is fine to panic on a Flush() to a non-batching producer
may be it is but it is still better if an error is returned, not panic.
--
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