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

refactor(daemon): rework power structure for easier persistence + platform abstraction #1797

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

kumavis
Copy link
Member

@kumavis kumavis commented Sep 29, 2023

trying to abstract over

  1. persistence mechanism
  2. runtime platform

@kumavis kumavis changed the title Kumavis/endo refactor powers refactor(daemon) - rework power structure for persistence abstraction Sep 29, 2023
@kumavis kumavis changed the base branch from master to endo September 29, 2023 06:41
@kumavis kumavis changed the title refactor(daemon) - rework power structure for persistence abstraction refactor(daemon) - rework power structure for easier persistence + platform abstraction Sep 29, 2023
getSha512Hex: () => Promise<string>;
}>;
makeHashedContentReadeableBlob: (statePath: string, sha512: string) => {
stream: () => Promise<FarRef<Reader<Uint8Array>>>;
Copy link
Member Author

Choose a reason for hiding this comment

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

is FarRef correct?

Copy link
Member

Choose a reason for hiding this comment

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

I think Promise<FarRef<T>> simplifies to FarRef<T> and it seems right to me. cc @michaelfig

Copy link
Member

Choose a reason for hiding this comment

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

If you intend for stream to be potentially remote but returned async, then you indeed need Promise<FarRef<T>>. If it's potentially remote but synchronously returned, you can get away with FarRef<T>.

packages/daemon/src/daemon-node-powers.js Show resolved Hide resolved
packages/daemon/src/daemon-node-powers.js Outdated Show resolved Hide resolved
packages/daemon/src/daemon-node-powers.js Outdated Show resolved Hide resolved
packages/daemon/src/daemon-node-powers.js Outdated Show resolved Hide resolved
packages/daemon/src/daemon-node-powers.js Show resolved Hide resolved
packages/daemon/src/daemon-node-powers.js Outdated Show resolved Hide resolved
packages/daemon/src/daemon-node-powers.js Outdated Show resolved Hide resolved
packages/daemon/src/daemon-node-powers.js Outdated Show resolved Hide resolved
packages/daemon/src/daemon.js Outdated Show resolved Hide resolved
getSha512Hex: () => Promise<string>;
}>;
makeHashedContentReadeableBlob: (statePath: string, sha512: string) => {
stream: () => Promise<FarRef<Reader<Uint8Array>>>;
Copy link
Member

Choose a reason for hiding this comment

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

I think Promise<FarRef<T>> simplifies to FarRef<T> and it seems right to me. cc @michaelfig

Copy link
Member

@kriskowal kriskowal left a comment

Choose a reason for hiding this comment

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

I am feeling good about the progress in this stack. Please feel free to squash these and assign the review to me when you think it’s ready for review. I’ve made some naming suggestions. Given that the project is far from stone, I’m happy to merge progress as long as it’s in a working state (as defined by the tests and a run-through of the demo (until we have a test for that!)).

packages/daemon/src/daemon-node-powers.js Outdated Show resolved Hide resolved
packages/daemon/src/daemon.js Outdated Show resolved Hide resolved
packages/daemon/src/daemon.js Outdated Show resolved Hide resolved
packages/daemon/src/pet-store.js Outdated Show resolved Hide resolved
packages/daemon/src/types.d.ts Show resolved Hide resolved
packages/daemon/src/daemon-node-powers.js Outdated Show resolved Hide resolved
@kumavis kumavis force-pushed the kumavis/endo-refactor-powers branch from 459521f to fc011ee Compare October 5, 2023 01:02
@kumavis

This comment was marked as outdated.

@kumavis

This comment was marked as resolved.

@kumavis

This comment was marked as resolved.

@kumavis kumavis mentioned this pull request Oct 11, 2023
7 tasks
@kumavis

This comment was marked as resolved.

@kriskowal kriskowal changed the title refactor(daemon) - rework power structure for easier persistence + platform abstraction refactor(daemon): rework power structure for easier persistence + platform abstraction Oct 13, 2023
packages/daemon/src/pet-store.js Show resolved Hide resolved
packages/daemon/src/types.d.ts Show resolved Hide resolved
@kumavis kumavis force-pushed the kumavis/endo-refactor-powers branch from 8249d19 to 3e10ebe Compare October 13, 2023 21:41
@kumavis kumavis marked this pull request as ready for review October 13, 2023 21:47
@kumavis kumavis merged commit 302679f into endo Oct 16, 2023
14 checks passed
@kumavis kumavis deleted the kumavis/endo-refactor-powers branch October 16, 2023 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants