Skip to content

Fix WriteBatch APIs and Update README to include Entry API #845

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

Merged
merged 4 commits into from
Jun 4, 2019

Conversation

ashish-goswami
Copy link
Contributor

@ashish-goswami ashish-goswami commented May 31, 2019

This change is Reviewable

@ashish-goswami ashish-goswami requested review from a team, jarifibrahim and mangalaman93 May 31, 2019 15:40
@ashish-goswami ashish-goswami force-pushed the ashish/writebatch-entryapi-fix branch 2 times, most recently from fb407c5 to 0ca58c6 Compare May 31, 2019 15:55
Copy link
Contributor

@jarifibrahim jarifibrahim left a comment

Choose a reason for hiding this comment

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

Looks like we do not have documentation for write batch API. We should add it in a separate PR.

Reviewed 8 of 8 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @ashish-goswami, @mangalaman93, and @manishrjain)


README.md, line 219 at r1 (raw file):

err := db.Update(func(txn *badger.Txn) error {
  e := NewEntry([]byte("answer"), []byte("42"))
  err := txn.SetEntry(e)

return txn.SetEntry(e)


README.md, line 346 at r1 (raw file):

err := db.Update(func(txn *badger.Txn) error {
  e := NewEntry([]byte("answer"), []byte("42")).WithTTL(time.Hour)
  err := txn.SetEntry(e)

return txn.SetEntry(e)


README.md, line 359 at r1 (raw file):

err := db.Update(func(txn *badger.Txn) error {
  e := NewEntry([]byte("answer"), []byte("42")).WithMeta(byte(0))
  err := txn.SetEntry(e)

return txn.SetEntry(e)


README.md, line 365 at r1 (raw file):

`Entry` APIs can be used to add the user metadata and TTL for same key. This `Entry`
then can be set using `Txn.SetEntry()`.

Can we add an example of withTTL and withMeta used together?

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

Defer to @jarifibrahim for final review. I got a comment.

Reviewed 8 of 8 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @ashish-goswami and @mangalaman93)


README.md, line 358 at r1 (raw file):

```go
err := db.Update(func(txn *badger.Txn) error {
  e := NewEntry([]byte("answer"), []byte("42")).WithMeta(byte(0))

Use byte(1) or 0xab or something to show meta being set. A zero meta is equivalent of not setting one.

Copy link
Contributor Author

@ashish-goswami ashish-goswami left a comment

Choose a reason for hiding this comment

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

Reviewable status: 7 of 8 files reviewed, 5 unresolved discussions (waiting on @jarifibrahim, @mangalaman93, and @manishrjain)


README.md, line 219 at r1 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

return txn.SetEntry(e)

I just wanted to have it simple here, hence wrote it in two lines.


README.md, line 346 at r1 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

return txn.SetEntry(e)

same reason.


README.md, line 358 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Use byte(1) or 0xab or something to show meta being set. A zero meta is equivalent of not setting one.

Done.


README.md, line 359 at r1 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

return txn.SetEntry(e)

same reason.


README.md, line 365 at r1 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

Can we add an example of withTTL and withMeta used together?

Done.

Copy link
Contributor

@jarifibrahim jarifibrahim left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @mangalaman93 and @manishrjain)

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @mangalaman93 and @manishrjain)

As we have removed SetWith functions from Txn struct. We can
do same for WriteBatch. Anyone can create Entry using new Entry
APIs and call SetEntry on WriteBatch.
@ashish-goswami ashish-goswami force-pushed the ashish/writebatch-entryapi-fix branch from 8cc45a6 to 824230d Compare June 4, 2019 08:45
@ashish-goswami ashish-goswami merged commit cd5884e into master Jun 4, 2019
@ashish-goswami ashish-goswami deleted the ashish/writebatch-entryapi-fix branch June 4, 2019 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants