Prepare for 0.4.0.M1 #61

smaldini · 2014-06-02T16:31:49Z

Docs tweaking and package info renaming in README.md

benjchristensen · 2014-06-02T17:36:02Z

Before we proceed with this the specs and readme need to be merged (as discussed on Twitter). We should not have to mentally merge two documents in order to have "the contract".

It also seems odd to have a spec for the TCK, it should be just be asserting the spec/contract, not creating a new one. It can have a README explaining itself, but I don't think it should have a spec.

viktorklang · 2014-06-02T17:38:15Z

@benjchristensen The reason for the spec residing in the TCK artifact was for the implementor to have access to an offline copy of the spec (since he'd use the TCK to verify his impl).

benjchristensen · 2014-06-02T17:41:01Z

It's not hard to have an offline copy of a single file meant for the API and TCK. I strongly believe we need a single document representing the contract, not two.

The TCK artifacts can easily include whatever the single file is.

smaldini · 2014-06-02T17:51:59Z

Have merged but my connection is down....

Sent from my iPhone

On 2 Jun 2014, at 18:41, Ben Christensen [email protected] wrote:

It's not hard to have an offline copy of a single file meant for the API and TCK. I strongly believe we need a single document representing the contract, not two.

The TCK artifacts can easily include whatever the single file is.

—
Reply to this email directly or view it on GitHub.

smaldini · 2014-06-02T17:56:07Z

Merged, discussions welcome.

benjchristensen · 2014-06-02T18:02:15Z

Thanks, I'll review ...

viktorklang · 2014-06-03T14:34:31Z

CopyrightWaivers.txt

@@ -21,3 +21,4 @@ github name    | Real Name, Email Address used for git commits, Company
 rkuhn          | Roland Kuhn, [email protected], Typesafe Inc.
 benjchristensen| Ben Christensen, [email protected], Netflix Inc.
 viktorklang    | Viktor Klang, [email protected], Typesafe Inc.
+smaldini       | Stephane Maldini, [email protected], Pivotal Software Inc. Inc.


remove double-Inc. :)

smaldini · 2014-06-03T14:59:32Z

BTW I didn't take time to explain how I merged, I just verified as much as I could duplicate rules and eliminate them then completed with rules that weren't present in README.md. Now it's up to us to decide whether some can go or not. Generally to me it sounds all right, I might wait for anyone else advice to double check the core behavior tho.

jbrisbin · 2014-06-03T17:36:35Z

I found having two different files confusing. We should explain the spec well enough that it's readable. IMO the spec should be the README and exist in only one place.

smaldini · 2014-06-03T17:38:38Z

README.md

@@ -73,6 +74,16 @@ public interface Subscriber<T> {
 - A `Subscriber` MUST NOT block a `Publisher` thread.
 - A `Subscriber` MUST signal demand via `Subscription.request` to receive notifications.
 - A `Subscriber` MAY behave synchronously or asynchronously but SHOULD NOT synchronously perform heavy computations in its methods (`onNext`, `onError`, `onComplete`, `onSubscribe`).
+- A `Subscriber.onNext(T t)` and `Subscriber.onSubscribe(Subscription s)` MUST NOT call any methods on the `Subscription`, the `Publisher` or any other `Publishers` or `Subscribers`.


I am not sure about that @viktorklang @benjchristensen @jbrisbin

If the Subscriber is allowed synchronous, then this would be a weird rule (to me).

benjchristensen · 2014-06-11T15:48:55Z

README.md

+| 10     | While the `Subscription` is not cancelled, `Subscription.request(int n)` MAY synchronously call `onNext` on this (or other) subscriber(s) |
+| 11     | While the `Subscription` is not cancelled, `Subscription.request(int n)` MAY synchronously call `onComplete` or `onError` on this (or other) subscriber(s) |
+| 12     | While the `Subscription` is not cancelled, `Subscription.cancel()` MUST request the `Publisher` to eventually stop signaling its `Subscriber`. The operation is NOT REQUIRED to affect the `Subscription` immediately. |
+| 13     | While the `Subscription` is not cancelled, `Subscription.cancel()` MUST request the `Publisher` to eventually drop any references to the corresponding subscriber. Re-subscribing with the same `Subscriber` object is discouraged [see 2.12], but this specification does not mandate that it is disallowed since that would mean having to store previously canceled subscriptions indefinitely |


Doesn't the portion about re-subscribing conflict with 2.12 that says onSubscribe can't be called more than once for a given Subscriber?

Yes, in the spec-violating range of behaviors we have a conflict, but since it is outside of the conforming range I propose that we resolve this issue later (i.e. ticket as in “known bug”, not “contentious”). I created #63.

smaldini · 2014-06-13T12:26:08Z

@reactive-streams/contributors - Most of the changes make sense here, that would be the last round (at least for this version). Before I commit the edited section as per @benjchristensen suggestions, let's focus on these last points, comment if wanted. It's not too late for releasing today :)

viktorklang · 2014-06-13T12:50:03Z

@rkuhn and I are going to comment today, we're in rural Germany w/o decent connectivity so we'll have to do that later during the day.

viktorklang · 2014-06-13T13:14:57Z

Just a heads up, we're not at consensus yet so hold off cutting the release.
After we reach consensus on this PR I suggest that we try to make individual PRs and discussions per suggested change so we don't end up with these massive changes.

smaldini · 2014-06-13T20:40:53Z

Ok merging and releasing... Kidding :) My suggestion is to go with the PR without the changes proposed here with the latest comments and continue the discussion on the remaining big changes separately. Minus some loosening in the rules, its pretty much tiding up and not that far from 0.3 either specially its TDK (we could even give a kick at the current one and change just a few things, add more verifications and use rule number in tests).

smaldini · 2014-06-13T20:44:31Z

So @reactive-streams/contributors should we have a consensus to merge the current branch, discuss release separately (or release now the simplified types as 0.4.0.M1), continue the discussion and release M2 with even fewer changes but closing outstanding tickets such as infinite request or precising capacity overflow rules.

jbrisbin · 2014-06-13T20:48:41Z

IMO we don't need complete agreement on all of the minor points to get moving forward. We can argue about the subtleties in subsequent issues related to specific areas of concern rather than holding up an entire codebase waiting for artifacts to be produced that never come due to the fact that we're never to going to agree on every detail.

👍

rkuhn · 2014-06-14T09:46:30Z

README.md

+| 10     | `Publisher.subscribe` SHOULD NOT throw a non-fatal  `Throwable`. The only legal way to signal failure (or reject a `Subscription`) is via the `onError` method. Non-fatal `Throwable` excludes any non-recoverable exception by the application (e.g. OutOfMemory) |
+| 12     | `Publisher.subscribe` MAY be called as many times as wanted but MUST be with a different `Subscriber` each time [see 2.12]. It MUST reject the `Subscription` with a `java.lang.IllegalStateException` if the same `Subscriber` already has an active `Subscription` with this `Publisher`. The cause message MUST include a reference to this rule and/or quote the full rule |
+| 13     | A `Publisher` MAY support multi-subscribe and choose whether each `Subscription` is unicast or multicast |
+| 14     | A `Publisher` MAY reject calls to its `subscribe` method if it is unable or unwilling to serve them (e.g. because it is overwhelmed or bounded by a finite number of underlying resources, etc...). If rejecting it MUST do this by calling `onError` on the `Subscriber` passed to `Publisher.subscribe` instead of calling `onSubscribe`" |


Please move the parenthesis out into a footnote and add more cases in which Subscribers may be rejected: when the Publisher cannot produce any more elements (exhausted a non-restartable source, shut down its resources; in general covering the cases from rules 15–17).

rkuhn · 2014-06-14T11:48:36Z

There are a few things which can be improved, as it entirely natural, so I agree with @jbrisbin that we can merge this PR as is. In keeping with the policy of doing small, clearly scoped changes one at a time I created tickets for outstanding fixes (#63, #64, #65, #66, #67, #68 and #69). This PR gives us a solid basis upon which to improve incrementally.

@mariusaeriksen @tmontgomery @purplefox @normanmaurer ?

…e to 1.15, 1.16, 1.17

smaldini · 2014-06-17T12:39:13Z

It is time :) @purplefox @mariusaeriksen @tmontgomery @normanmaurer - I need your plus one to release as it so we can progress on #63, #64, #65, #66, #67, #68 and #69 ....

tmontgomery · 2014-06-17T14:50:27Z

👍

benjchristensen · 2014-06-17T17:28:29Z

I'm okay with merging, but not releasing. The items listed above need to be resolved before releasing, and there are other open discussions without issues created:

#61 (comment)

+| 7 | A Subscriber MUST ensure that all calls on its Subscription take place from the same thread or provide for respective external synchronization |

#61 (comment)

+| 11 | A Subscriber MUST make sure that all calls on its onXXX methods happen-before [1] the processing of the respective signals. I.e. the Subscriber must take care of properly publishing the signal to its processing logic |

#61 (comment)

+| 2 | The Subscription MUST allow the Subscriber to call Subscription.request synchronously from within onNext or onSubscribe |

#61 (comment)

+| 14 | While the Subscription is not cancelled, invoking Subscription.cancel MAY cause the Publisher to transition into the shut-down state if no other Subscription exists at this point [see 1.17].

#61 (comment)

+| 3 | A Processor MAY choose to recover an onError signal. If it chooses to do so, it MUST consider the Subscription canceled, otherwise it MUST propagate the onError signal to its Subscribers immediately |

smaldini · 2014-06-17T22:37:59Z

As long as we don't have TCK, I don't see why we can't release in current state the 4 simplified classes. For sure the doc is going to receive several amendments until M2 or RC1 but that will drive the TCK work mainly which needs the M1 to start. I think its a solid base, apart from request(-1) I don't see a lot changing in the API jar artifact to release yet. WDYT ? I think the current README.md is a good starting point as all issues/comments are directly related to that base now (and there isn't a tck specific rules documents anymore).

benjchristensen · 2014-06-18T04:48:21Z

Sorry, I mistook "release" to mean more ... if it's just releasing the jar as an artifact, I see no issue with that, it should have been done weeks ago, since 0.3 is already out there and it's wrong.

So I'm +1 on merging this, releasing code as 0.4, and then continuing to resolve the outstanding issues and discussions each on their own (with smaller PRs).

normanmaurer · 2014-06-18T04:50:06Z

@smaldini unfortunately I'm quite too busy atm to follow this stuff at all :( So don't wait on me, I trust you guys ....

Prepare for 0.4.0.M1

smaldini · 2014-06-18T08:03:49Z

Moving forward then :)

Stephane Maldini and others added 5 commits June 2, 2014 13:38

Edit README.md

8b4d958

Adding me (Stephane Maldini) in cop. waivers

af380eb

Update CopyrightWaivers.txt

092302b

Update README.md

5bab96e

Reintroduce spec.md

0d11049

smaldini mentioned this pull request Jun 2, 2014

Release version 0.4.0 #56

Closed

Precise Processor behaviors

cf09f74

Merge spec.MD with README.md

98a06e5

viktorklang reviewed Jun 3, 2014
View reviewed changes

Remove double Inc.

1ae8ba0

smaldini reviewed Jun 3, 2014
View reviewed changes

benjchristensen reviewed Jun 11, 2014
View reviewed changes

rkuhn reviewed Jun 14, 2014
View reviewed changes

Move parenthesis from Publisher rule 1.14 to note .1 and add referenc…

f709200

…e to 1.15, 1.16, 1.17

smaldini added a commit that referenced this pull request Jun 18, 2014

Merge pull request #61 from reactive-streams/edit-doc-simplified-types

7a15ab6

Prepare for 0.4.0.M1

smaldini merged commit 7a15ab6 into master Jun 18, 2014

rkuhn mentioned this pull request Mar 3, 2020

Not calling Subscription methods in onComplete #481

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare for 0.4.0.M1 #61

Prepare for 0.4.0.M1 #61

smaldini commented Jun 2, 2014

benjchristensen commented Jun 2, 2014

viktorklang commented Jun 2, 2014

benjchristensen commented Jun 2, 2014

smaldini commented Jun 2, 2014

smaldini commented Jun 2, 2014

benjchristensen commented Jun 2, 2014

viktorklang Jun 3, 2014

smaldini Jun 3, 2014

smaldini commented Jun 3, 2014

jbrisbin commented Jun 3, 2014

smaldini Jun 3, 2014

viktorklang Jun 3, 2014

smaldini Jun 3, 2014

benjchristensen Jun 11, 2014

rkuhn Jun 14, 2014

smaldini commented Jun 13, 2014

viktorklang commented Jun 13, 2014

viktorklang commented Jun 13, 2014

smaldini commented Jun 13, 2014

smaldini commented Jun 13, 2014

jbrisbin commented Jun 13, 2014

rkuhn Jun 14, 2014

rkuhn commented Jun 14, 2014

smaldini commented Jun 17, 2014

tmontgomery commented Jun 17, 2014

benjchristensen commented Jun 17, 2014

smaldini commented Jun 17, 2014

benjchristensen commented Jun 18, 2014

normanmaurer commented Jun 18, 2014

smaldini commented Jun 18, 2014

Prepare for 0.4.0.M1 #61

Prepare for 0.4.0.M1 #61

Conversation

smaldini commented Jun 2, 2014

benjchristensen commented Jun 2, 2014

viktorklang commented Jun 2, 2014

benjchristensen commented Jun 2, 2014

smaldini commented Jun 2, 2014

smaldini commented Jun 2, 2014

benjchristensen commented Jun 2, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smaldini commented Jun 3, 2014

jbrisbin commented Jun 3, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smaldini commented Jun 13, 2014

viktorklang commented Jun 13, 2014

viktorklang commented Jun 13, 2014

smaldini commented Jun 13, 2014

smaldini commented Jun 13, 2014

jbrisbin commented Jun 13, 2014

Choose a reason for hiding this comment

rkuhn commented Jun 14, 2014

smaldini commented Jun 17, 2014

tmontgomery commented Jun 17, 2014

benjchristensen commented Jun 17, 2014

smaldini commented Jun 17, 2014

benjchristensen commented Jun 18, 2014

normanmaurer commented Jun 18, 2014

smaldini commented Jun 18, 2014