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

tree: Import / Export APIs and demo #22566

Open
wants to merge 85 commits into
base: main
Choose a base branch
from

Conversation

CraigMacomber
Copy link
Contributor

Description

Added import/export options for tree content and schema, and example script using them.

Reviewer Guidance

The review process is outlined on this wiki page.

CraigMacomber and others added 30 commits July 25, 2024 12:26
@CraigMacomber CraigMacomber changed the title tree: Import / Export demo tree: Import / Export APIs and demo Oct 24, 2024
@CraigMacomber CraigMacomber marked this pull request as ready for review October 24, 2024 19:25
@CraigMacomber CraigMacomber requested review from a team as code owners October 24, 2024 19:25
Copy link
Contributor

@jzaffiro jzaffiro left a comment

Choose a reason for hiding this comment

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

A few minor comments, but docs look good!

.changeset/chubby-olives-say.md Outdated Show resolved Hide resolved
.changeset/chubby-olives-say.md Outdated Show resolved Hide resolved
.changeset/chubby-olives-say.md Outdated Show resolved Hide resolved
examples/apps/tree-cli-app/README.md Outdated Show resolved Hide resolved
@CraigMacomber CraigMacomber requested a review from a team as a code owner October 29, 2024 20:26
CraigMacomber added a commit that referenced this pull request Oct 29, 2024
…#22927)

## Description

A collection of minor improvements split out from
#22566 for simplified
and more targeted review.
.changeset/chubby-olives-say.md Outdated Show resolved Hide resolved
const args = process.argv.slice(2);

console.log(`Requires arguments: [<source>] [<destination>] [<edit>]`);
console.log(`Example arguments: default data/large.concise.json string:10,item:100`);
Copy link
Contributor

Choose a reason for hiding this comment

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

the example for edit is a bit confusing to me, maybe expand on its expected format or how this particular example affects the document?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should be better now.

examples/apps/tree-cli-app/src/schema.ts Outdated Show resolved Hide resolved
examples/apps/tree-cli-app/src/test/schema.spec.ts Outdated Show resolved Hide resolved
examples/apps/tree-cli-app/src/test/schema.spec.ts Outdated Show resolved Hide resolved
},
...options,
};
const schemalessConfig = applySchemaToParserOptions(schema, config);
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this considered schemaless?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

comment added.

const codec = makeFieldBatchCodec({ jsonValidator: noopValidator }, format);
const cursor = borrowFieldCursorFromTreeNodeOrValue(node);
const batch: FieldBatch = [cursor];
// If none provided, create a compressor which will not compress anything (TODO: is this the right way to do that?).
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems like a todo that should prob be addressed before merging

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@taylorsw04 Can you provide some inside here and on the TODO below?

packages/dds/tree/src/shared-tree/treeApiAlpha.ts Outdated Show resolved Hide resolved
packages/dds/tree/src/simple-tree/api/tree.ts Outdated Show resolved Hide resolved
@@ -0,0 +1 @@
{"type":"com.fluidframework.example.cli.List","fields":[]}
Copy link
Contributor

Choose a reason for hiding this comment

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

is the verbose version supposed to not have any fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the default tree does not have any children under this node, so yes. I have however changed the default tree to be a bit bigger so its a better example.

@@ -67,6 +67,13 @@ export interface ViewableTree {
viewWith<TRoot extends ImplicitFieldSchema>(
config: TreeViewConfiguration<TRoot>,
): TreeView<TRoot>;

// TODO:
// Add stored key versions of Tree.exportVerbose, Tree.exportConcise and Tree.exportCompressed here so tree content can be accessed without a view schema.
Copy link
Contributor

Choose a reason for hiding this comment

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

link here for context would be good I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have moved these comments to the interface doc comment so they can use the link syntax. They are in privateRemarks though, so I don't think there is any way to click them, and they don't render as links in intelisense.

Copy link
Contributor

github-actions bot commented Nov 1, 2024

🔗 Found some broken links! 💔

Run a link check locally to find them. See
https://github.com/microsoft/FluidFramework/wiki/Checking-for-broken-links-in-the-documentation for more information.

linkcheck output


> [email protected] ci:linkcheck /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test ci:start 1313 linkcheck:full

1: starting server using command "npm run ci:start"
and when url "[ 'http://127.0.0.1:1313' ]" is responding with HTTP status code 200
running tests using command "npm run linkcheck:full"


> [email protected] ci:start
> http-server ./public --port 1313 --silent


> [email protected] linkcheck:full
> npm run linkcheck:fast -- --external


> [email protected] linkcheck:fast
> linkcheck http://localhost:1313 --skip-file skipped-urls.txt --external

Crawling...

http://localhost:1313/docs/api/v1/protocol-definitions/
- (3127:9) 'JSON Web..' => https://jwt.io/introduction/ (connection failed)

http://localhost:1313/docs/api/v1/protocol-definitions/itokenclaims-interface/
- (2873:3) 'JSON Web..' => https://jwt.io/introduction/ (connection failed)
- (2873:3) 'JSON Web..' => https://jwt.io/introduction/ (connection failed)

http://localhost:1313/docs/api/v2/driver-definitions/
- (3602:9) 'JSON Web..' => https://jwt.io/introduction/ (connection failed)
- (3602:9) 'JSON Web..' => https://jwt.io/introduction/ (connection failed)

http://localhost:1313/docs/api/v2/driver-definitions/itokenclaims-interface/
- (3307:3) 'JSON Web..' => https://jwt.io/introduction/ (connection failed)
- (3307:3) 'JSON Web..' => https://jwt.io/introduction/ (connection failed)

http://localhost:1313/docs/api/v2/protocol-definitions/
- (3482:9) 'JSON Web..' => https://jwt.io/introduction/ (connection failed)

http://localhost:1313/docs/api/v2/protocol-definitions/itokenclaims-interface/
- (3307:3) 'JSON Web..' => https://jwt.io/introduction/ (connection failed)
- (3307:3) 'JSON Web..' => https://jwt.io/introduction/ (connection failed)

http://localhost:1313/docs/api/v2/routerlicious-driver/itokenresponse-interface/
- (3328:8) 'JSON Web..' => https://jwt.io/introduction/ (connection failed)
- (3341:8) 'JSON Web..' => https://jwt.io/introduction/ (connection failed)
- (3328:8) 'JSON Web..' => https://jwt.io/introduction/ (connection failed)
- (3341:8) 'JSON Web..' => https://jwt.io/introduction/ (connection failed)


Stats:
  440610 links
    3397 destination URLs
       2 URLs ignored
      14 warnings
       1 errors

 ELIFECYCLE  Command failed with exit code 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dds: tree area: dds Issues related to distributed data structures area: examples Changes that focus on our examples area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct base: main PRs targeted against main branch changeset-present dependencies Pull requests that update a dependency file public api change Changes to a public API size regression Significant bundle size regression (>5 KB)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants