Skip to content
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

Fix for SharedTree op named idAllocation #21867

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

arafat-java
Copy link
Contributor

Description

Fixes #21866

When fetch-tool is used to fetch all the ops it fails to fetch block of type="idAllocation" associated with SharedTree
This op usually comes with sequenceNumber=2 with Brainstorm sample app

More details around error log and reproduction steps can be found in bug/issue #21866

@github-actions github-actions bot added the base: main PRs targeted against main branch label Jul 12, 2024
@msfluid-bot
Copy link
Collaborator

@fluid-example/bundle-size-tests: +245 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 457.26 KB 457.3 KB +35 Bytes
azureClient.js 555.05 KB 555.1 KB +49 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 258.33 KB 258.35 KB +14 Bytes
fluidFramework.js 392.16 KB 392.17 KB +14 Bytes
loader.js 134.1 KB 134.11 KB +14 Bytes
map.js 42.17 KB 42.17 KB +7 Bytes
matrix.js 145.56 KB 145.57 KB +7 Bytes
odspClient.js 522.82 KB 522.86 KB +49 Bytes
odspDriver.js 97.17 KB 97.19 KB +21 Bytes
odspPrefetchSnapshot.js 42.27 KB 42.29 KB +14 Bytes
sharedString.js 162.65 KB 162.66 KB +7 Bytes
sharedTree.js 382.62 KB 382.63 KB +7 Bytes
Total Size 3.27 MB 3.27 MB +245 Bytes

Baseline commit: 38edc9b

Generated by 🚫 dangerJS against 1ba9bee

@@ -607,8 +607,8 @@ function processOp(
type: string;
};
const address = envelope.address;
type = `${type}/${innerContent.type}`;
switch (innerContent.type) {
type = `${type}/${innerContent?.type}`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking this over, all this code only applies to FluidDataStoreOp. I'd pull the other 3 to their own case chunk and have it do nothing or whatever makes sense.

You'll be able to remove several as's (e.g. as IEnvelope on line 600) this way too, because the types express the type of contents

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And revert the addition of ?

@markfields
Copy link
Member

This PR has been automatically marked as stale because it has had no activity for 60 days. It will be closed if no further activity occurs within 8 days of this comment. Thank you for your contributions to Fluid Framework!

Friendly ping, @arafat-java, in case you want to come back to this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
base: main PRs targeted against main branch community-contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fetch-tool fails to work with SharedTree ops
4 participants