read-log doesn't end when using tx ids

[jeremyheiler]

If you use tx ids for start and end with read-log, it never ends since it compares the end-tx with t, which is (effectively) always smaller.

https://github.com/onyx-platform/onyx-datomic/blob/0.10.x/src/onyx/plugin/datomic.clj#L200

This leads me to wonder if tx ids are supported? I assumed so based on the name of the keys :datomic/log-start-tx and :datomic/log-end-tx. And both t and tx ids are supported by datomic.api/tx-range.

I'd be happy to provide a patch either way.

[MichaelDrogalis]

cc @lbradstreet

[lbradstreet]

Hi Jeremy, Yes, this is a bug. We are trying to be consistent with datomic's tx-range call which is inclusive of the start tx and exclusive of the end tx, but I think it makes far more sense to be inclusive of both the start and the end. This would be simpler to understand and use and is more consistent with the other streaming plugins. I agree that supporting Tx ids would be a good addition. If you'd like to supply a PR for both of these we'd be very happy to merge it. Thanks!

…

On 24 Mar 2017, at 08:52, Michael Drogalis ***@***.***> wrote: cc @lbradstreet — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[lbradstreet]

Hi @jeremyheiler, we're still happy to put this in onyx-datomic if you've made any progress. Thanks!

[jeremyheiler]

I'm working on it now. I had to take a break from this, but now I'm back and it's directly blocking me.

[lbradstreet]

Great. I didn't mean to be pushy! Cheers.

…

On 4 April 2017 at 10:40, Jeremy Heiler ***@***.***> wrote: Will do! I had to take a break from this, but now I'm back and it's directly blocking me. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#26 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPZHU16b_X9Zk2xTXFN6oT3WbGipVCGks5rsoB9gaJpZM4MoZ4e> .

[jeremyheiler]

No worries! :-)

[jeremyheiler]

Thinking about it a little more, I'm not sure the best way to handle tx ids. My thought would be to normalize them to t values so comparison operations can be done. But is there a good predicate for tx ids? Perhaps byte length?

[lbradstreet]

Normalizing to t values seems reasonable.

Maybe we could make a breaking change and deprecate:
:datomic/log-start-tx and :daotmic/log-end-tx.

and then create two sets of task parameters
:datomic/log-start-t, :datomic/log-end-t
and
:datomic/log-start-txid, :datomic/log-end-txid

Then we could also change the behaviour of log-end-t to not be exclusive on the end t?

Alternatively, the task-map could take a param :datomic/log-tx-type :txid or :datomic/log-tx-type :t which switches between them.

[jeremyheiler]

I don't think I have a preference. In the first solution, someone could accidentally mix :datomic/log-start-t with :datomic/log-end-txid. Is there a place to validate that? Although, it'd be easy to normalize the one txid to a t, so that might be OK. Then again, with the second solution, someone could say :txid and provide t values. Bleh..