remove requirement to track demand beyond Long.MAX_VALUE #203
Conversation
| | <a name="3.14">14</a> | While the `Subscription` is not cancelled, invoking `Subscription.cancel` MAY cause the `Publisher`, if stateful, to transition into the `shut-down` state if no other `Subscription` exists at this point [see [1.13](#1.13)]. | | ||
| | <a name="3.15">15</a> | `Subscription.cancel` MUST NOT throw an `Exception` and MUST signal `onError` to its `Subscriber`. | | ||
| | <a name="3.16">16</a> | `Subscription.request` MUST NOT throw an `Exception` and MUST signal `onError` to its `Subscriber`. | | ||
| | <a name="3.17">17</a> | A `Subscription` MUST support an unbounded number of calls to request and MUST support a demand (sum requested - sum delivered) up to 2^63-1 (`java.lang.Long.MAX_VALUE`). A demand equal or greater than 2^63-1 (`java.lang.Long.MAX_VALUE`) MAY be considered by the `Publisher` as “effectively unbounded”[[1](#footnote-3-1)]. | |
There was a problem hiding this comment.
This is the only rule line with more than a simple “full stop” change.
There was a problem hiding this comment.
Doesn't the TCK need to get updated as well?
There was a problem hiding this comment.
Seems like this PR should remove required_spec317_mustSignalOnErrorWhenPendingAboveLongMaxValue now, that it may just ignore additional requests after MAX_VALUE is reached.
There was a problem hiding this comment.
Ping @rkuhn RE TCK update
There was a problem hiding this comment.
Sorry for the delay, the TCK update is pushed now as a separate commit.
|
LGTM |
|
I suspect @benjchristensen will like this (IIRC he proposed this change a while back?) |
|
@reactive-streams/contributors Please comment/vote/give thumbs up before the 23rd of January. |
|
+1 On Tue, Jan 20, 2015 at 2:06 PM, Viktor Klang (√) [email protected]
Stéphane |
|
👍 |
|
Does that mean we ignore all together the request or we switch to the Long.MAX behavior in that case ? |
|
@smaldini Typically I'd say implementations will add the requested number to current demand and cap currentDemand at Long.MAX_VALUE |
|
So @viktorklang effectively turning a Subscription into unbounded mode amIright? |
|
I guess that can be left unspecified. |
fixes #196 Also terminate spec rule sentences with a full stop where missing.
|
All updated including a TCK test that fails if onError is signaled after demand-overflowing request calls. There are several +1 already on this PR, when should we merge? |
|
example publisher and tck changes - LGTM 👍 |
|
Yep, comments were due before the 23rd, merging! |
remove requirement to track demand beyond Long.MAX_VALUE
fixes #196
Also terminate spec rule sentences with a full stop where missing.