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

Iterator API #65

Merged
merged 3 commits into from Oct 24, 2012
Merged

Iterator API #65

merged 3 commits into from Oct 24, 2012

Conversation

yrashk
Copy link
Contributor

@yrashk yrashk commented Oct 24, 2012

This change allows applications that need something more controllable than a "blind fold" to iterate over bitcask keydir keys.

@slfritchie
Copy link
Contributor

I don't see a reason not to do this. Opinions @gburd or @justinsheehy or @jonmeredith?

@gburd
Copy link

gburd commented Oct 24, 2012

@slfritchie agreed, +1 to merge

@yrashk
Copy link
Contributor Author

yrashk commented Oct 24, 2012

If necessary, I can also add iterator, iterator_next and iterator_release wrappers in bitcask module that will extract keydir out of #bc_state{}

@yrashk
Copy link
Contributor Author

yrashk commented Oct 24, 2012

(since bc_state is effectively opaque)

@slfritchie
Copy link
Contributor

Yurii, the extractors sound good, er, or even necessary. I guess it becomes the user's responsibility to not do anything dumb with that state. You could (in theory) go meddling with the keydir and break things in countless ways. But if that's what you want to do, that's rope that I'm willing to give. Any disagreement, oh Bashonians?

@yrashk
Copy link
Contributor Author

yrashk commented Oct 24, 2012

I am working on wrappers already.

@jonmeredith
Copy link
Contributor

I'd personally like to see the wrappers. Bitcask should present a single-module interface rather than have people digging in the process dictionary and working out the NIF arguments.

(just saw you say you were working on wrappers as I wrote it).

@yrashk
Copy link
Contributor Author

yrashk commented Oct 24, 2012

Added wrappers

@yrashk
Copy link
Contributor Author

yrashk commented Oct 24, 2012

Thoughts? Can we merge this? I would love to be able to use this instead of playing with an opaque structure stored in my process' dictionary...

@justinsheehy
Copy link
Contributor

+1

Merging.

justinsheehy added a commit that referenced this pull request Oct 24, 2012
@justinsheehy justinsheehy merged commit 15ab02e into basho:master Oct 24, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants