Skip to content

feat(kv): Asyncify key-value#59

Merged
adamreese merged 2 commits intospinframework:mainfrom
adamreese:feat/key-value-async
Apr 30, 2026
Merged

feat(kv): Asyncify key-value#59
adamreese merged 2 commits intospinframework:mainfrom
adamreese:feat/key-value-async

Conversation

@adamreese
Copy link
Copy Markdown
Contributor

@adamreese adamreese commented Apr 16, 2026

Refactor key-value to use new async wit bindings.
GetKeys() now returns an iterator to allow the user to process them concurrently.

Signed-off-by: Adam Reese <adam@reese.io>
@adamreese adamreese requested a review from dicej April 16, 2026 18:02
Comment thread kv/kv.go Outdated
@@ -78,12 +78,27 @@ func (s *Store) Exists(key string) (bool, error) {

// GetKeys returns all the keys from the store.
func (s *Store) GetKeys() ([]string, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks like it won't allow async traversal - it seems to collect the whole lot in memory rather than allowing processing one at a time?

Or am I misunderstanding Go slices?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Taking a second look at this, I think this should return (chan string, error) instead.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

An iterator is likely the right API choice here for modern idiomatic go, rather than forcing consumers to manage pulling from a channel themselves.

The unfortunate part of moving KV to channels tho is that it's a... not great perf tradeoff as a default/only option for collections

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, an iterator makes sense. It worked out fine here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@endocrimes I haven't spent much time with iter. Would this be the way to approach it? adamreese@32a5d8e. I can apply the changes to this PR for easier review if you think its sane.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried a new approach returning iter.Seq2[string, error]

dicej
dicej previously approved these changes Apr 16, 2026
@dicej dicej self-requested a review April 17, 2026 13:44
@dicej dicej dismissed their stale review April 17, 2026 13:46

Realized that GetKeys should have a different return type.

Signed-off-by: Adam Reese <adam@reese.io>
Copy link
Copy Markdown
Collaborator

@dicej dicej left a comment

Choose a reason for hiding this comment

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

Thanks!

@adamreese adamreese merged commit 9cf514f into spinframework:main Apr 30, 2026
2 checks passed
@adamreese adamreese deleted the feat/key-value-async branch April 30, 2026 20:49
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