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

feat: add durable storage MVP #267

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

feat: add durable storage MVP #267

wants to merge 1 commit into from

Conversation

MasterPtato
Copy link
Contributor

@MasterPtato MasterPtato commented May 10, 2024 •
edited by Blckbrry-Pi

Resolves OGBE-97

@MasterPtato Graphite App
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @MasterPtato and the rest of your teammates on Graphite Graphite

@MasterPtato MasterPtato force-pushed the 05-10-feat_add_durable_storage_mvp branch from f6e97b3 to e4cabfa Compare May 13, 2024 23:38
Blckbrry-Pi
Blckbrry-Pi requested changes May 15, 2024
View reviewed changes
Copy link
Collaborator

@Blckbrry-Pi Blckbrry-Pi left a comment

Choose a reason for hiding this comment

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

Looks great overall!

I don't know too much about durable storage and the idea behind it, but I think the Deno implementation is pretty severely lacking without persistence.

Additionally, most of the comments I left on the Deno impl also apply to the CF impl.

Again, I apologize for advocating for more typescript complexity, but if instantiating a Durable it isn't disallowed by the type system, we NEED to explicitly document that. Module authors will definitely do that by accident.

Additionally, I'm not completely sure what the "point" of the durable storage is. Is that not what postgres and prisma are for?

src/runtime/durable.ts Outdated Show resolved Hide resolved
src/runtime/context.ts Outdated Show resolved Hide resolved
tests/basic/modules/foo/scripts/call_self.ts Outdated Show resolved Hide resolved
src/runtime/durable_cf.ts Outdated Show resolved Hide resolved
src/runtime/durable.ts Outdated Show resolved Hide resolved
src/runtime/durable.ts Outdated Show resolved Hide resolved
src/runtime/durable.ts Outdated Show resolved Hide resolved
@linear Linear
Copy link

linear bot commented May 15, 2024

OGBE-97 OpenGB Durable Storage

@NathanFlurry
Copy link
Member

@MasterPtato would you rather me wait until the next iteration is ready to go or review this now?

@MasterPtato MasterPtato marked this pull request as draft May 16, 2024 16:58
@MasterPtato Graphite App
Copy link
Contributor Author

@Blckbrry-Pi @NathanFlurry this was meant to be a draft sorry

@MasterPtato MasterPtato force-pushed the 05-10-feat_add_durable_storage_mvp branch from e4cabfa to c6d5e6e Compare May 16, 2024 21:40
@MasterPtato MasterPtato marked this pull request as ready for review May 16, 2024 22:20
@MasterPtato MasterPtato force-pushed the 05-10-feat_add_durable_storage_mvp branch from c6d5e6e to 4ff9bac Compare May 16, 2024 22:23
@Blckbrry-Pi Blckbrry-Pi force-pushed the 05-10-feat_add_durable_storage_mvp branch from 4ff9bac to 832f88c Compare May 21, 2024 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NathanFlurry NathanFlurry Awaiting requested review from NathanFlurry

@Blckbrry-Pi Blckbrry-Pi Awaiting requested review from Blckbrry-Pi

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@MasterPtato @NathanFlurry @Blckbrry-Pi