Merged
Conversation
Changing to "postgresql" as the official Postgres type as the parser library currently requires that format and there are no other dependencies on the alternative.
Update how values are passed into `executeTransaction` so LiteREST calls succeed
Added CORS check back in and updated JWKS logic to support auth requests with JWT
Fix up export and import functionality for internal sources
Needs additional testing before official support
Brayden
approved these changes
Dec 21, 2024
Member
Brayden
left a comment
There was a problem hiding this comment.
I've added a few follow-up commits after testing all of the features. Overall this PR is a quality of life improvements introducing these high-level items, but many more (see original description) included as well.
- Type safety
- Ability to toggle on/off features
- Shared RPC reference for reduced costs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
As discussed on Discord, this is a pretty large change to allow full configurability of Starbase and overhauls some of the internals.
Key changes:
DataSource.rpcas a single RPC Session, this means for the duration of a single request, we're only charged for a single DO request, whereas currently it's multiple.StarbaseDB.DataSourceinstance, with:rpcproperty. This is the result of calling the DOinit()method to return an RPC session.source, eitherexternalorinternal. For worker deployed users, this is still set via theX-Starbase-Sourceheader.cacheboolean property - works is set totrueand source isexternal. For worker deployed users, this is still set via theX-Starbase-Cacheheader istrue.contextproperty, which is what the RLS rules use. For worker deployed users, decodes the JWT and puts in the payload data. But for custom implementation, they can get the context from wherever they want.externalproperty, which accepts aExternalDatabaseSourcetype, which has all of our providers nicely typed.expireCachelogic has been moved into the internal handler.role, of eitheradminorclient. Admin role does not apply allow list or RLS rules. For the deployed worker, this is set by observing the env api keys which are set.Some additional notes / details:
new_sqlite_classes = ["StarbaseDBDurableObject"]? Do we have to tag the migration somehow?sourceon the URL when connecting via websocket... Shouldn't this be per query on the message level?I've roughly tested things work.. but with no testing I'm conscious this is a big PR with lots of room to go wrong 👀