EditableTree: sequences#12488
Closed
sharptrip wants to merge 11 commits into
Closed
Conversation
github-actions
Bot
added
area: dds
| /** | ||
| * Sequence field nodes as a sequence of EditableTrees or primitive values (if unwrapped). | ||
| */ | ||
| export interface EditableTreeSequence<T extends EditableTreeOrPrimitive = EditableTreeOrPrimitive> |
Contributor
There was a problem hiding this comment.
One way I think we could simplify the users of this type, is to remove the generic type parameter.
Instead of getting a EditableTreeSequence or a EditableTreeSequence, we can have a single sequence type, which will give you UnwrappedEditiableTrees by default, but we can add an API to get an EditiableTree explicitly (ex: getWithoutUnwraping(index: number): EditiableTree).
| import { AdaptingProxyHandler, getArrayOwnKeys, keyIsValidIndex } from "./utilities"; | ||
|
|
||
| /** | ||
| * Sequence field nodes as a sequence of EditableTrees or primitive values (if unwrapped). |
Contributor
There was a problem hiding this comment.
If you get an EditableTreeSequence by iterating an EditiableTree node, it might not correspond to a sequence field: you could get one for some other type of field like a value field.
Since all our fields have a schema and field key (including the root, though its field key is a detached sequence id, that still can be used as a LocalFieldKey for it), I think it might make sense to add a FieldSchema and FieldKey fields to this type, and rename it EditableField (replacing the existing EditiableField type).
I think that would help simplify the API, while fixing the name of this type at the same time.
…yEvent (microsoft#12482) Today, the ElectedClientNotSummarized event is treated as an error and as the number of hits is dependent to the number of running clients, a running stress test with 100+ clients will end up generating 100+ errors. This is not necessarily a bug and ends up creating unnecessary incidents (Ex. [Bug 2241](https://dev.azure.com/fluidframework/internal/_workitems/edit/2241)). After talking to the OCEs (Navin, Si) and Jatin, we've decided to move it from an error to a generic telemetry event to reduce the noise from the system.
…ft#12472) This PR converts all `export *` in `packages/drivers` to named exports. Related issue: microsoft#10062 See for the fact that bundle size does not change when the entire repo is converted: microsoft#12321 (comment).
This PR converts all `export *` in `/tools` to named exports. Related issue: microsoft#10062 See for the fact that bundle size does not change when the entire repo is converted: microsoft#12321 (comment).
…oft#12424) This PR converts `export *` in `package/framework` to named exports. Related issue: microsoft#10062 See for the fact that bundle size does not change when the entire repo is converted: microsoft#12321 (comment).
…rosoft#12399) This PR converts default exports in `experimental/PropertyDDS` to named exports. Related issue: microsoft#10062 See for the fact that bundle size does not change when the entire repo is converted: microsoft#12321 (comment).
The previous change in microsoft#11670 was incomplete. It did not properly pass through the build tools location from the ADO UI all the way to the right places. This change corrects that. I tested by manually running a build with the "next" value for the build tools source, and verified that the prerelease (tagged `@next`) version was installed.
Through @fluidframework/azure-scenario-runner we are exploring improvements allowing us to build, execute and validate azure-client scenarios in more flexible and effective manner. At this point in time, we will mostly use the scenario runner with FRS team to validate service characteristics. However, as we learn further, we should be able to use it beyond service validation.
AgentScheduler exposes handle but not via IFluidLoadable which makes it hard to access in general.
## Background As-is, the Sweep Timeout will always be at least 6 days due to hardcoded Snapshot Cache limit (5 days) and Buffer (1 day). This means it's impossible to test the Sweep Timer and the code that runs after something is SweepReady! There have been multiple attempts on my part to address this: microsoft#11084, microsoft#12044, microsoft#12331 Much of the difficulty has arisen from the challenge of aligning driver policy with GC config. I'm finally punting on that and tracking it as a separate item ([AB#2238](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/2238)). ## This change It's pretty simple: * Persist SweepTimeoutMs in the summary * For new containers, that can be overridden via the `"Fluid.GarbageCollection.TestOverride.SweepTimeoutMs"` config setting * Default behavior for new containers (and for backfilling existing containers with no value persisted) is SweepTimeout + 6 days (same as it has been)
### Description 665: Ability to Rerun Publish Step If There Are Any Network Error https://dev.azure.com/fluidframework/internal/_workitems/edit/665/ ### Cases 1. If Publish Package does not encounter an error continue with the remaining packages 2. If Publish Package encounters an error - Network Error: retry for `$maximumRetryIfNetworkError`-amount of time. If network error is persisted during the entire loop, exit the task in the pipeline - Non-Network Error: Such as the current package is already published in the registry, fail the pipeline and not continue
github-actions
Bot
added
area: build
CraigMacomber
added a commit
that referenced
this pull request
Oct 27, 2022
Same as [#12488](#12512), but using a new cursor API. ## Description This PR implements `EditableField` as an array-like sequence of nodes (`EditableTrees`), which 1) replaces handling of sequence fields with arrays, 2) allows to access nodes lazily by their indices with and without unwrapping, and 3) is a required change to implement editing of `EditableTree`. ## Reviewer Guidance Everything around "root" probably might be considered as a workaround as currently `cursor.getPath()` and anchors do not support fields. Also, it seems that "primaryField" property of `FieldProxyTarget` is redundant, as `fieldKey` is also there, but it adds clarity, and we might make use of it when implementing editing. ## Other information or known dependencies **Step 4** in a series to push code from #12182 to enable editing of EditableTree. [Step 3](#12428) [Step 2](#12306) [Step 1](#12300) Co-authored-by: Craig Macomber (Microsoft) <42876482+CraigMacomber@users.noreply.github.com>
sharptrip
added a commit
to sharptrip/FluidFramework
that referenced
this pull request
Oct 28, 2022
) Same as [microsoft#12488](microsoft#12512), but using a new cursor API. ## Description This PR implements `EditableField` as an array-like sequence of nodes (`EditableTrees`), which 1) replaces handling of sequence fields with arrays, 2) allows to access nodes lazily by their indices with and without unwrapping, and 3) is a required change to implement editing of `EditableTree`. ## Reviewer Guidance Everything around "root" probably might be considered as a workaround as currently `cursor.getPath()` and anchors do not support fields. Also, it seems that "primaryField" property of `FieldProxyTarget` is redundant, as `fieldKey` is also there, but it adds clarity, and we might make use of it when implementing editing. ## Other information or known dependencies **Step 4** in a series to push code from microsoft#12182 to enable editing of EditableTree. [Step 3](microsoft#12428) [Step 2](microsoft#12306) [Step 1](microsoft#12300) Co-authored-by: Craig Macomber (Microsoft) <42876482+CraigMacomber@users.noreply.github.com>
alexvy86
pushed a commit
to alexvy86/FluidFramework
that referenced
this pull request
Nov 1, 2022
) Same as [microsoft#12488](microsoft#12512), but using a new cursor API. ## Description This PR implements `EditableField` as an array-like sequence of nodes (`EditableTrees`), which 1) replaces handling of sequence fields with arrays, 2) allows to access nodes lazily by their indices with and without unwrapping, and 3) is a required change to implement editing of `EditableTree`. ## Reviewer Guidance Everything around "root" probably might be considered as a workaround as currently `cursor.getPath()` and anchors do not support fields. Also, it seems that "primaryField" property of `FieldProxyTarget` is redundant, as `fieldKey` is also there, but it adds clarity, and we might make use of it when implementing editing. ## Other information or known dependencies **Step 4** in a series to push code from microsoft#12182 to enable editing of EditableTree. [Step 3](microsoft#12428) [Step 2](microsoft#12306) [Step 1](microsoft#12300) Co-authored-by: Craig Macomber (Microsoft) <42876482+CraigMacomber@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces EditableTreeSequence and its proxy-based implementation, which replaces unwrapping sequence fields into arrays in EditableTree. This is required to enable editing of EditableTree.
Reviewer Guidance
I'd like to have some preliminary feedback on the overall concept, and maybe more detailed on the following:
EditableTreeSequenceandProxyTargetSequenceare typedProxyTarget.isEmpty)Other information or known dependencies
Step 4 in a series to push code from #12182 to enable editing of EditableTree.
Step 3
Step 2
Step 1