-
Notifications
You must be signed in to change notification settings - Fork 1
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
Implement the latest changes to room lifecycle spec #66
Labels
enhancement
New feature or improved functionality.
room-lifecycle
Related to room lifecycle (temporary label).
Comments
lawrence-forooghian
added a commit
that referenced
this issue
Sep 26, 2024
Implements points CHA-RL4* from same spec as referenced in e70ee44. TODO btw i haven't put action descriptions in all of the test names because they might end up really long In addition to the TODOs added in the code (all of which refer either to existing GitHub issues or questions on the spec, for which we have #66 as a catch-all issue), I’ve also not done: - CHA-RL4a2 — I don’t understand the meaning of “has not yet successfully managed to attach its Realtime Channel”, asked about it in [1] - CHA-RL4b2 — seems redundant, asked about it in [2] - CHA-RL4b3, CHA-RL4b4 — seem redundant, asked about it in [3] - CHA-RL4b5, CHA-RL4b6, CHA-RL4b7 — these relate to transient disconnect timeouts, so will do them in #48 Resolves #53. (TODO move to next commit with the discontinuity points) [1] https://github.com/ably/specification/pull/200/files#r1775552624 [2] https://github.com/ably/specification/pull/200/files#r1777212960 [3] https://github.com/ably/specification/pull/200/files#r1777365677
lawrence-forooghian
added a commit
that referenced
this issue
Sep 26, 2024
Implements points CHA-RL4* from same spec as referenced in e70ee44. TODO btw i haven't put action descriptions in all of the test names because they might end up really long In addition to the TODOs added in the code (all of which refer either to existing GitHub issues or questions on the spec, for which we have #66 as a catch-all issue), I’ve also not done: - CHA-RL4a2 — I don’t understand the meaning of “has not yet successfully managed to attach its Realtime Channel”, asked about it in [1] - CHA-RL4b2 — seems redundant, asked about it in [2] - CHA-RL4b3, CHA-RL4b4 — seem redundant, asked about it in [3] - CHA-RL4b5, CHA-RL4b6, CHA-RL4b7 — these relate to transient disconnect timeouts, so will do them in #48 Resolves #53. (TODO move to next commit with the discontinuity points) [1] https://github.com/ably/specification/pull/200/files#r1775552624 [2] https://github.com/ably/specification/pull/200/files#r1777212960 [3] https://github.com/ably/specification/pull/200/files#r1777365677
lawrence-forooghian
added a commit
that referenced
this issue
Sep 30, 2024
Implements points CHA-RL4* from same spec as referenced in e70ee44. TODO btw i haven't put action descriptions in all of the test names because they might end up really long In addition to the TODOs added in the code (all of which refer either to existing GitHub issues or questions on the spec, for which we have #66 as a catch-all issue), I’ve also not done: - CHA-RL4a2 — I don’t understand the meaning of “has not yet successfully managed to attach its Realtime Channel”, asked about it in [1] - CHA-RL4b2 — seems redundant, asked about it in [2] - CHA-RL4b3, CHA-RL4b4 — seem redundant, asked about it in [3] - CHA-RL4b5, CHA-RL4b6, CHA-RL4b7 — these relate to transient disconnect timeouts, so will do them in #48 Resolves #53. (TODO move to next commit with the discontinuity points) [1] https://github.com/ably/specification/pull/200/files#r1775552624 [2] https://github.com/ably/specification/pull/200/files#r1777212960 [3] https://github.com/ably/specification/pull/200/files#r1777365677
lawrence-forooghian
added a commit
that referenced
this issue
Sep 30, 2024
Implements points CHA-RL4* from same spec as referenced in e70ee44. TODO btw i haven't put action descriptions in all of the test names because they might end up really long In addition to the TODOs added in the code (all of which refer either to existing GitHub issues or questions on the spec, for which we have #66 as a catch-all issue), I’ve also not done: - CHA-RL4a2 — I don’t understand the meaning of “has not yet successfully managed to attach its Realtime Channel”, asked about it in [1] - CHA-RL4b2 — seems redundant, asked about it in [2] - CHA-RL4b3, CHA-RL4b4 — seem redundant, asked about it in [3] - CHA-RL4b5, CHA-RL4b6, CHA-RL4b7 — these relate to transient disconnect timeouts, so will do them in #48 Part of #53. [1] https://github.com/ably/specification/pull/200/files#r1775552624 [2] https://github.com/ably/specification/pull/200/files#r1777212960 [3] https://github.com/ably/specification/pull/200/files#r1777365677
lawrence-forooghian
added a commit
that referenced
this issue
Oct 1, 2024
Implements points CHA-RL4* from same spec as referenced in e70ee44. In addition to the TODOs added in the code (all of which refer either to existing GitHub issues or questions on the spec, for which we have #66 as a catch-all issue), I’ve also not done: - CHA-RL4a2 — I don’t understand the meaning of “has not yet successfully managed to attach its Realtime Channel”, asked about it in [1] - CHA-RL4b2 — seems redundant, asked about it in [2] - CHA-RL4b3, CHA-RL4b4 — seem redundant, asked about it in [3] - CHA-RL4b5, CHA-RL4b6, CHA-RL4b7 — these relate to transient disconnect timeouts, so will do them in #48 Something which I didn’t think about in 25e5052, and which I haven’t thought about here, is how actor reentrancy (i.e. the fact that when an actor-isolated method suspends via `await`, another actor-isolated method can be scheduled instead, which can potentially cause issues like state updates being interleaved in unexpected ways) might affect the room lifecycle manager. I would like to first of all implement the whole thing, specifically all of the spec points that might provide us with a mutual exclusion mechanism (i.e. the ones that tell us to wait until the current operation is complete), before assessing the situation. Have created #75 for this. Aside: I have not been consistent with the way that I’ve named the tests; the existing lifecycle manager test names have a part that describes the expected side effects. But I haven’t done that here because some of the spec points tested here have multiple side effects and the test names would become really long and hard to read. So for those ones I’ve only described the expected side effects inside the tests. I think we can live with the inconsistency for now. Part of #53. [1] https://github.com/ably/specification/pull/200/files#r1775552624 [2] https://github.com/ably/specification/pull/200/files#r1777212960 [3] https://github.com/ably/specification/pull/200/files#r1777365677
lawrence-forooghian
added a commit
that referenced
this issue
Oct 1, 2024
Implements points CHA-RL4* from same spec as referenced in e70ee44. In addition to the TODOs added in the code (all of which refer either to existing GitHub issues or questions on the spec, for which we have #66 as a catch-all issue), I’ve also not done: - CHA-RL4a2 — I don’t understand the meaning of “has not yet successfully managed to attach its Realtime Channel”, asked about it in [1] - CHA-RL4b2 — seems redundant, asked about it in [2] - CHA-RL4b3, CHA-RL4b4 — seem redundant, asked about it in [3] - CHA-RL4b5, CHA-RL4b6, CHA-RL4b7 — these relate to transient disconnect timeouts, so will do them in #48 Something which I didn’t think about in 25e5052, and which I haven’t thought about here, is how actor reentrancy (i.e. the fact that when an actor-isolated method suspends via `await`, another actor-isolated method can be scheduled instead, which can potentially cause issues like state updates being interleaved in unexpected ways) might affect the room lifecycle manager. I would like to first of all implement the whole thing, specifically all of the spec points that might provide us with a mutual exclusion mechanism (i.e. the ones that tell us to wait until the current operation is complete), before assessing the situation. Have created #75 for this. Aside: I have not been consistent with the way that I’ve named the tests; the existing lifecycle manager test names have a part that describes the expected side effects. But I haven’t done that here because some of the spec points tested here have multiple side effects and the test names would become really long and hard to read. So for those ones I’ve only described the expected side effects inside the tests. I think we can live with the inconsistency for now. Part of #53. [1] https://github.com/ably/specification/pull/200/files#r1775552624 [2] https://github.com/ably/specification/pull/200/files#r1777212960 [3] https://github.com/ably/specification/pull/200/files#r1777365677
lawrence-forooghian
added a commit
that referenced
this issue
Oct 1, 2024
Implements points CHA-RL4* from same spec as referenced in e70ee44. In addition to the TODOs added in the code (all of which refer either to existing GitHub issues or questions on the spec, for which we have #66 as a catch-all issue), I’ve also not done: - CHA-RL4a2 — I don’t understand the meaning of “has not yet successfully managed to attach its Realtime Channel”, asked about it in [1] - CHA-RL4b2 — seems redundant, asked about it in [2] - CHA-RL4b3, CHA-RL4b4 — seem redundant, asked about it in [3] - CHA-RL4b5, CHA-RL4b6, CHA-RL4b7 — these relate to transient disconnect timeouts, so will do them in #48 Something which I didn’t think about in 25e5052, and which I haven’t thought about here, is how actor reentrancy (i.e. the fact that when an actor-isolated method suspends via `await`, another actor-isolated method can be scheduled instead, which can potentially cause issues like state updates being interleaved in unexpected ways) might affect the room lifecycle manager. I would like to first of all implement the whole thing, specifically all of the spec points that might provide us with a mutual exclusion mechanism (i.e. the ones that tell us to wait until the current operation is complete), before assessing the situation. Have created #75 for this. Aside: I have not been consistent with the way that I’ve named the tests; the existing lifecycle manager test names have a part that describes the expected side effects. But I haven’t done that here because some of the spec points tested here have multiple side effects and the test names would become really long and hard to read. So for those ones I’ve only described the expected side effects inside the tests. I think we can live with the inconsistency for now. Part of #53. [1] https://github.com/ably/specification/pull/200/files#r1775552624 [2] https://github.com/ably/specification/pull/200/files#r1777212960 [3] https://github.com/ably/specification/pull/200/files#r1777365677
lawrence-forooghian
added a commit
that referenced
this issue
Oct 3, 2024
Implements points CHA-RL4* from same spec as referenced in e70ee44. In addition to the TODOs added in the code (all of which refer either to existing GitHub issues or questions on the spec, for which we have #66 as a catch-all issue), I’ve also not done: - CHA-RL4a2 — I don’t understand the meaning of “has not yet successfully managed to attach its Realtime Channel”, asked about it in [1] - CHA-RL4b2 — seems redundant, asked about it in [2] - CHA-RL4b3, CHA-RL4b4 — seem redundant, asked about it in [3] - CHA-RL4b5, CHA-RL4b6, CHA-RL4b7 — these relate to transient disconnect timeouts, so will do them in #48 Something which I didn’t think about in 25e5052, and which I haven’t thought about here, is how actor reentrancy (i.e. the fact that when an actor-isolated method suspends via `await`, another actor-isolated method can be scheduled instead, which can potentially cause issues like state updates being interleaved in unexpected ways) might affect the room lifecycle manager. I would like to first of all implement the whole thing, specifically all of the spec points that might provide us with a mutual exclusion mechanism (i.e. the ones that tell us to wait until the current operation is complete), before assessing the situation. Have created #75 for this. Aside: I have not been consistent with the way that I’ve named the tests; the existing lifecycle manager test names have a part that describes the expected side effects. But I haven’t done that here because some of the spec points tested here have multiple side effects and the test names would become really long and hard to read. So for those ones I’ve only described the expected side effects inside the tests. I think we can live with the inconsistency for now. Part of #53. [1] https://github.com/ably/specification/pull/200/files#r1775552624 [2] https://github.com/ably/specification/pull/200/files#r1777212960 [3] https://github.com/ably/specification/pull/200/files#r1777365677
lawrence-forooghian
added a commit
that referenced
this issue
Oct 8, 2024
Implements points CHA-RL4* from same spec as referenced in e70ee44. In addition to the TODOs added in the code (all of which refer either to existing GitHub issues or questions on the spec, for which we have #66 as a catch-all issue), I’ve also not done: - CHA-RL4a2 — I don’t understand the meaning of “has not yet successfully managed to attach its Realtime Channel”, asked about it in [1] - CHA-RL4b2 — seems redundant, asked about it in [2] - CHA-RL4b3, CHA-RL4b4 — seem redundant, asked about it in [3] - CHA-RL4b5, CHA-RL4b6, CHA-RL4b7 — these relate to transient disconnect timeouts, so will do them in #48 Something which I didn’t think about in 25e5052, and which I haven’t thought about here, is how actor reentrancy (i.e. the fact that when an actor-isolated method suspends via `await`, another actor-isolated method can be scheduled instead, which can potentially cause issues like state updates being interleaved in unexpected ways) might affect the room lifecycle manager. I would like to first of all implement the whole thing, specifically all of the spec points that might provide us with a mutual exclusion mechanism (i.e. the ones that tell us to wait until the current operation is complete), before assessing the situation. Have created #75 for this. Created [4] to address the `@preconcurrency import Ably` introduced by this commit. Aside: I have not been consistent with the way that I’ve named the tests; the existing lifecycle manager test names have a part that describes the expected side effects. But I haven’t done that here because some of the spec points tested here have multiple side effects and the test names would become really long and hard to read. So for those ones I’ve only described the expected side effects inside the tests. I think we can live with the inconsistency for now. Part of #53. [1] https://github.com/ably/specification/pull/200/files#r1775552624 [2] https://github.com/ably/specification/pull/200/files#r1777212960 [3] https://github.com/ably/specification/pull/200/files#r1777365677 [4] ably/ably-cocoa#1987
lawrence-forooghian
added a commit
that referenced
this issue
Oct 8, 2024
Implements points CHA-RL4* from same spec as referenced in e70ee44. In addition to the TODOs added in the code (all of which refer either to existing GitHub issues or questions on the spec, for which we have #66 as a catch-all issue), I’ve also not done: - CHA-RL4a2 — I don’t understand the meaning of “has not yet successfully managed to attach its Realtime Channel”, asked about it in [1] - CHA-RL4b2 — seems redundant, asked about it in [2] - CHA-RL4b3, CHA-RL4b4 — seem redundant, asked about it in [3] - CHA-RL4b5, CHA-RL4b6, CHA-RL4b7 — these relate to transient disconnect timeouts, so will do them in #48 Something which I didn’t think about in 25e5052, and which I haven’t thought about here, is how actor reentrancy (i.e. the fact that when an actor-isolated method suspends via `await`, another actor-isolated method can be scheduled instead, which can potentially cause issues like state updates being interleaved in unexpected ways) might affect the room lifecycle manager. I would like to first of all implement the whole thing, specifically all of the spec points that might provide us with a mutual exclusion mechanism (i.e. the ones that tell us to wait until the current operation is complete), before assessing the situation. Have created #75 for this. Created [4] to address the `@preconcurrency import Ably` introduced by this commit. Aside: I have not been consistent with the way that I’ve named the tests; the existing lifecycle manager test names have a part that describes the expected side effects. But I haven’t done that here because some of the spec points tested here have multiple side effects and the test names would become really long and hard to read. So for those ones I’ve only described the expected side effects inside the tests. I think we can live with the inconsistency for now. Part of #53. [1] https://github.com/ably/specification/pull/200/files#r1775552624 [2] https://github.com/ably/specification/pull/200/files#r1777212960 [3] https://github.com/ably/specification/pull/200/files#r1777365677 [4] ably/ably-cocoa#1987
lawrence-forooghian
changed the title
Implement outcomes of pending questions on spec
Implement the latest changes to spec
Oct 29, 2024
lawrence-forooghian
changed the title
Implement the latest changes to spec
Implement the latest changes to room lifecycle spec
Oct 29, 2024
lawrence-forooghian
added a commit
that referenced
this issue
Nov 6, 2024
Based on the spec referenced in 20f21c7. The RETRY part of this spec was quite unclear, and I asked quite a few questions on the PR to understand it better, so the behaviour implemented here is based on the spec plus Andy’s answers to my questions (I’ve linked to the discussions in the code and / or tests). Recently (i.e. after most of this commit was already implemented, Andy has updated the spec with answers to these questions, but in the interests of not dragging out the current task, I’ll incorporate these updates in #66. The internal triggering of the RETRY operation (as specified by CHA-RL1h3 and CHA-RL4b9) will come in #50. Resolves #51.
lawrence-forooghian
added
the
room-lifecycle
Related to room lifecycle (temporary label).
label
Nov 6, 2024
This was referenced Nov 6, 2024
lawrence-forooghian
added a commit
that referenced
this issue
Nov 12, 2024
We use the implementation of the RELEASE operation provided by the room lifecycle manager, and implement the spec points relating to room map bookkeeping and releasing the underlying realtime channels. Based on [1] at 6f0740a. I have not implemented the spec points that relate to making sure that a room fetch waits for any previous room with the same ID to finish releasing; this is a part of the spec which is in flux (currently implemented via the INITIALIZING status, which was added to the spec after we started implementing the room lifecycle manager and hasn’t been implemented in this SDK yet, and soon to be further changed in the spec by making room-getting async). We can look at the current state of things when we come to do #66. [1] ably/specification#200
lawrence-forooghian
added a commit
that referenced
this issue
Nov 12, 2024
We use the implementation of the RELEASE operation provided by the room lifecycle manager, and implement the spec points relating to room map bookkeeping and releasing the underlying realtime channels. Based on [1] at 6f0740a. I have not implemented the spec points that relate to making sure that a room fetch waits for any previous room with the same ID to finish releasing; this is a part of the spec which is in flux (currently implemented via the INITIALIZING status, which was added to the spec after we started implementing the room lifecycle manager and hasn’t been implemented in this SDK yet, and soon to be further changed in the spec by making room-getting async). We can look at the current state of things when we come to do #66. Part of #47. [1] ably/specification#200
lawrence-forooghian
added a commit
that referenced
this issue
Nov 12, 2024
We use the implementation of the RELEASE operation provided by the room lifecycle manager, and implement the spec points relating to room map bookkeeping and releasing the underlying realtime channels. Based on [1] at 6f0740a. I have not implemented the spec points that relate to making sure that a room fetch waits for any previous room with the same ID to finish releasing; this is a part of the spec which is in flux (currently implemented via the INITIALIZING status, which was added to the spec after we started implementing the room lifecycle manager and hasn’t been implemented in this SDK yet, and soon to be further changed in the spec by making room-getting async). We can look at the current state of things when we come to do #66. Part of #47. [1] ably/specification#200
lawrence-forooghian
added a commit
that referenced
this issue
Nov 12, 2024
We use the implementation of the RELEASE operation provided by the room lifecycle manager, and implement the spec points relating to room map bookkeeping and releasing the underlying realtime channels. Based on [1] at 6f0740a. I have not implemented the spec points that relate to making sure that a room fetch waits for any previous room with the same ID to finish releasing; this is a part of the spec which is in flux (currently implemented via the INITIALIZING status, which was added to the spec after we started implementing the room lifecycle manager and hasn’t been implemented in this SDK yet, and soon to be further changed in the spec by making room-getting async). We can look at the current state of things when we come to do #66. Part of #47. [1] ably/specification#200
lawrence-forooghian
added a commit
that referenced
this issue
Nov 12, 2024
Based on the spec referenced in 20f21c7. The RETRY part of this spec was quite unclear, and I asked quite a few questions on the PR to understand it better, so the behaviour implemented here is based on the spec plus Andy’s answers to my questions (I’ve linked to the discussions in the code and / or tests). Recently (i.e. after most of this commit was already implemented, Andy has updated the spec with answers to these questions, but in the interests of not dragging out the current task, I’ll incorporate these updates in #66. The internal triggering of the RETRY operation (as specified by CHA-RL1h3 and CHA-RL4b9) will come in #50. Resolves #51.
lawrence-forooghian
added a commit
that referenced
this issue
Nov 12, 2024
WIP updating to be based on 0e5ab98 Based on the spec referenced in 20f21c7. The RETRY part of this spec was quite unclear, and I asked quite a few questions on the PR to understand it better, so the behaviour implemented here is based on the spec plus Andy’s answers to my questions (I’ve linked to the discussions in the code and / or tests). Recently (i.e. after most of this commit was already implemented, Andy has updated the spec with answers to these questions, but in the interests of not dragging out the current task, I’ll incorporate these updates in #66. The internal triggering of the RETRY operation (as specified by CHA-RL1h3 and CHA-RL4b9) will come in #50. Resolves #51.
This was referenced Nov 26, 2024
This was referenced Nov 27, 2024
Open
Figured out what needs to be done to get in line with spec version I’ve opened #156 which implements a bunch of spec alignment, and split the remaining work into the following issues:
I’ll close the current issue now. |
lawrence-forooghian
closed this as not planned
Won't fix, can't repro, duplicate, stale
Nov 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
enhancement
New feature or improved functionality.
room-lifecycle
Related to room lifecycle (temporary label).
INITIALIZING
state). See what's new and implement.DETACH
operation, not just the channel detach┆Issue is synchronized with this Jira Story by Unito
The text was updated successfully, but these errors were encountered: