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.

Sign up for GitHub

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

Jump to bottom

Prototype migration with generic dataobject #2

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

Prototype migration with generic dataobject #2

wants to merge 10 commits into from

Conversation

tyler-cai-microsoft
Copy link
Owner

@tyler-cai-microsoft tyler-cai-microsoft commented Aug 17, 2023 •
edited
Loading

DLehenbauer
DLehenbauer reviewed Aug 22, 2023
View reviewed changes
packages/test/test-end-to-end-tests/src/test/dataMigration.spec.ts
* runtime already is established and it can communicate back to the runtime factory to tell it to create a migration summarizer.
*
* A thought I'm concerned about is that the search blob can disappear for all dataObjects that are modified, though search will
* resume on the next summarization (probably).
Copy link

Choose a reason for hiding this comment

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

Since the app implements the DataMigrationObject, perhaps the app can regenerate and include the search blob as part of the migration step?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah, I think they can definitely do that.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think the thing I worry about is that they have their own implementation of the fluid runtime, it would be nice if they didn't need to do that.

I think it would be better if we provided a better API where they just call into the datastore runtime to pregenerate the search blob

@github-actions
Copy link

github-actions bot commented Sep 6, 2023

🔗 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


> fluidframework-docs@0.25.0 ci:linkcheck /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test ci:start http://localhost:1313 linkcheck:full

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


> fluidframework-docs@0.25.0 ci:start
> http-server ./public --port 1313 --silent


> fluidframework-docs@0.25.0 linkcheck:full
> npm run linkcheck:fast -- --external


> fluidframework-docs@0.25.0 linkcheck:fast
> linkcheck http://localhost:1313 --skip-file skipped-urls.txt --external

Crawling...

http://localhost:1313/docs/apis/runtime-definitions/
- (1703:37) 'https://..' => https://datatracker.ietf.org/doc/html/rfc4122%29 (HTTP 404)
- (1703:37) 'https://..' => https://datatracker.ietf.org/doc/html/rfc4122%29 (HTTP 404)

http://localhost:1313/docs/build/bundlers/
- (843:233) 'here' => https://github.com/microsoft/FluidFramework/blob/a4c38234a920abe9b54b1c26a14c0a8e430cd3fa/packages/tools/webpack-fluid-loader/webpack.config.js#L37 (HTTP 200 but missing anchor)

http://localhost:1313/docs/deployment/service-options/
- (848:72) 'Routerli..' => https://github.com/microsoft/FluidFramework/tree/main/server#readme (HTTP 200 but missing anchor)
- (850:168) 'Routerli..' => https://github.com/microsoft/FluidFramework/tree/main/server#readme (HTTP 200 but missing anchor)

http://localhost:1313/docs/faq/
- (927:87) 'Routerli..' => https://github.com/microsoft/FluidFramework/tree/main/server#readme (HTTP 200 but missing anchor)

http://localhost:1313/docs/testing/telemetry/
- (848:3) 'ILoaderP..' => https://github.com/microsoft/FluidFramework/blob/main/packages/loader/container-loader/src/loader.ts#L313 (HTTP 200 but missing anchor)


Summary of most serious issues:

http://localhost:1313/docs/apis/runtime-definitions/
- (1703:37) 'https://..' => https://datatracker.ietf.org/doc/html/rfc4122%29 (HTTP 404)
- (1703:37) 'https://..' => https://datatracker.ietf.org/doc/html/rfc4122%29 (HTTP 404)


Stats:
  209119 links
    2038 destination URLs
       2 URLs ignored
       5 warnings
       1 errors

 ELIFECYCLE  Command failed with exit code 1.

tyler-cai-microsoft pushed a commit that referenced this pull request Apr 16, 2024
@vladsud
Fix dehydrate tests (attempt #2): Use proper versions of DDS, DataSto…
442cb76 
…re, runtime, Loader in versioned tests (microsoft#20650)

An extraction from microsoft#20621

Reason for changes
These tests explode when I change interface between DDS & DataStore

Description
These tests are not written correctly. They mix arbitrary version of DDS with the latest version of FluidDataStoreRuntime (through TestFluidObjectFactory). We never supported such mixes - version of DDS has to match version or data store!

Using matching TestFluidObjectFactory is easy, but then we start running into somewhat similar problem of mixing FluidDataStoreRuntime & ContainerRuntime. We support only N/N-1 mixes here, and nothing more. So we have to use the version provided by test framework, thus you see some dance above CodeLoader to get right factory.

And while we are at it, it would be great for the test to use Loader version provided by test framework, not latest version.
This breaks for old versions as the way test is written it works only for latest structure of the document. Previously test framework was running a ton of combos, but most of them were actually the same, as even when it was supplied different versions of loader, we were using only latest version of loader. So, the right fix here - use loader version provided by framework but skip all the tests that use old version of the loader, thus getting same result, but with fewer iterations (and faster tests).

As part of fixing it, use proper back-compat compatible ways to get to entry points at loader & runtime layers.
tyler-cai-microsoft pushed a commit that referenced this pull request May 13, 2024
@CraigMacomber
Allow more cases in Tree.is (microsoft#20973)
6dae7eb 
## Description

This makes 3 changes to Tree.is:
1. calling Tree.is(node, SchemaNotUsedInTreeSchema) is no longer an
error. This undocumented edge case could have been problematic, and is
inconsistent since it would not fire if node was not a TreeNode.
2. Tree.is now takes in ImplicitAllowedTypes, making cases like
Tree.is(x, [schema.number, schema.string]) valid. THis is more
performant and more concise then doing two separate checks and ORing
them together. This also allows checking a node against an
ImplicitAllowedTypes pulled from a FieldSchema which could be handy for
some generic code.
3. The implementation of Tree.schema and Tree.is have been rewritten to
not rely on flex-schema as much, and fast path non-TreeNode inputs. This
should make it more maintainable and more performant.


Interestingly `#2` above is the only case that couldn't be covered by
`instanceof` (assuming TypeScript 5.3): we could make `instanceof` do
all narrowing currently done with Tree.is, except for this new case. The
presence of this case thus seems to motivate keeping `Tree.is` if for no
reason other than it can support this additional pattern which
`instancof` cannot.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DLehenbauer DLehenbauer DLehenbauer left review comments

Assignees
No one assigned
Labels
area: build area: contributor experience area: dds: propertydds area: dds: sharedstring area: dds: tree area: dds area: dev experience area: driver area: examples area: framework area: loader area: odsp-driver area: runtime area: server area: tests area: website base: main community-contribution dependencies public api change
Projects
None yet
Milestone
No milestone
2 participants
@tyler-cai-microsoft @DLehenbauer