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

SubscribeToAllTables, which hides "SELECT * FROM *" #211

Merged
merged 7 commits into from
Jan 14, 2025

Conversation

gefjon
Copy link
Contributor

@gefjon gefjon commented Jan 10, 2025

Description of Changes

Per out-of-band discussion, add SubscriptionBuilder.SubscribeToAllTables, which abstracts over SubscriptionBuilder.Subscribe(["SELECT * FROM *"]).

In the future, we will change the implementation of this method, so that it uses "legacy subscriptions" while SubscriptionBuilder.Subscribe moves to using "mutable subscriptions." At that time, no other interface will be provided for using "legacy subscriptions." SubscribeToAllTables may also at some point be rewritten in terms of "mutable subscriptions" somehow.

API

  • This is an API breaking change to the SDK

If the API is breaking, please state below what will break

Requires SpacetimeDB PRs

N/a

Testsuite

If you would like to run the your SDK changes in this PR against a specific SpacetimeDB branch, specify that here. This can be a branch name or a link to a PR.

SpacetimeDB branch name: master

Testing

Write instructions for a test that you performed for this PR

In the future, we will change the implementation of this method,
so that it uses "legacy subscriptions"
while `SubscriptionBuilder.Subscribe` moves to using "mutable subscriptions."
At that time, no other interface will be provided for using "legacy subscriptions."
`SubscribeToAllTables` may also at some point
be rewritten in terms of "mutable subscriptions" somehow.
@gefjon gefjon changed the base branch from master to staging January 10, 2025 21:00
Copy link
Collaborator

@cloutiertyler cloutiertyler left a comment

Choose a reason for hiding this comment

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

The change is good. I will investigate the failing tests and see if we can't fix those.

@bfops
Copy link
Collaborator

bfops commented Jan 13, 2025

The CI here should be fixed once #210 merges.

That PR should be merged once we've pushed NuGet versions of the BSATN DLLs.

Co-authored-by: Ingvar Stepanyan <[email protected]>
@bfops
Copy link
Collaborator

bfops commented Jan 14, 2025

CI now failing due to whitespace errors

@cloutiertyler
Copy link
Collaborator

Although the cost was high (see #217) I have verified that this change is working appropriately in the tutorial.

@cloutiertyler cloutiertyler enabled auto-merge (squash) January 14, 2025 23:29
@cloutiertyler cloutiertyler merged commit bb22fcc into staging Jan 14, 2025
6 checks passed
@cloutiertyler cloutiertyler deleted the phoebe/subscribe-to-all-tables branch January 14, 2025 23:43
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.

4 participants