search

vedang / clj_fdb

A thin Clojure wrapper for the Java API for FoundationDB.
https://vedang.github.io/clj_fdb/
Eclipse Public License 1.0
25 stars 9 forks source link

tuple io #26

Closed manish-2014 closed 3 years ago

manish-2014 commented 3 years ago

I had run into an issue with string serialization https://github.com/vedang/clj_fdb/issues/25 and fixing byte-array issues is complicated. Also, it makes sense to defer mapping the value until the value is absolutely needed . Also added "get-future" because sometimes we don't need the value immediately.

vedang commented 3 years ago

Hello! I have not had the chance to review this yet, but I'll make some time for it today/tomorrow. Sorry to leave you waiting without any feedback, thank you for your patience on this :)

vedang commented 3 years ago

I still haven't looked at this due to other work commitments, I'll make time for it tonight. Comments from a cursory glance:

  1. The indentation in this PR is terrible. Please use cljstyle to fix the indentation in these files.
  2. I'm not sure that the internal/util file has moved from src to test in this PR, on first glance this seems wrong and will break the rest of the code.

Thanks for adding this functionality and tests too! I'll review them thoroughly and get back to you asap.

vedang commented 3 years ago

At the moment, I do not think that these changes are necessary. My understanding it that you have written this code to solve your problem of serialization / deserialization with byte-arrays, but it should not be part of the library. I'll close this in favor of implementing #17

Also, re get-future, the future should be realized within the context of the run call, so that the transaction can be committed to the DB.