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

Merged
merged 1 commit into from
May 25, 2024
Merged

feat: add durable storage MVP #267

merged 1 commit into from
May 25, 2024

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

MasterPtato commented May 10, 2024 •
edited

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 previously 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
@NathanFlurry NathanFlurry force-pushed the 05-10-feat_add_durable_storage_mvp branch from 832f88c to ec33168 Compare May 25, 2024 03:22
@NathanFlurry NathanFlurry changed the base branch from main to 05-24-fix_server_update_request_handler_to_work_for_cloudflare_workers May 25, 2024 03:22
@NathanFlurry NathanFlurry mentioned this pull request May 25, 2024
Merged
@NathanFlurry
Copy link
Member

@MasterPtato merging this since it's blocking other things. let's make sure this gets cleaned up at some point soon.

@NathanFlurry Graphite App
Copy link
Member

NathanFlurry commented May 25, 2024 •
edited

Merge activity

  • May 24, 11:41 PM EDT: @NathanFlurry started a stack merge that includes this pull request via Graphite.
  • May 24, 11:42 PM EDT: Graphite rebased this pull request as part of a merge.
  • May 24, 11:43 PM EDT: @NathanFlurry merged this pull request with Graphite.

Base automatically changed from 05-24-fix_server_update_request_handler_to_work_for_cloudflare_workers to main May 25, 2024 03:41
@NathanFlurry NathanFlurry force-pushed the 05-10-feat_add_durable_storage_mvp branch from ec33168 to 75fd0dc Compare May 25, 2024 03:42
@NathanFlurry NathanFlurry merged commit 7d2d7be into main May 25, 2024
2 of 4 checks passed
@NathanFlurry NathanFlurry deleted the 05-10-feat_add_durable_storage_mvp branch May 25, 2024 03:43
@github-actions github-actions bot mentioned this pull request May 25, 2024
Open
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

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