search

aerospike / aerospike-client-rust

Rust client for the Aerospike database
https://www.aerospike.com/
Other
82 stars 29 forks source link

Add APIs to wait for completion of long-running server tasks #69

Closed jlr52 closed 4 years ago

jlr52 commented 4 years ago

This is a proof of concept for adding query status api similar to https://github.com/aerospike/aerospike-client-java/blob/master/client/src/com/aerospike/client/task/IndexTask.java

I have a few questions that I listed as TODO (ie. what is the appropriate error)

TODOs before possible merge:

Add more test

jhecking commented 4 years ago

Thanks for this contribution, @jlr52!

I think this is a good, first step towards resolving #12. What would be needed next is the equivalent of the Java client's [Task class], which can be broken down into two parts: 1) A common Task interface, that all tasks like the IndexTask would implement (public abstract int queryStatus() in the Java client's Task implementation). 2) The common logic to take any Task instance, and repeated query the tasks status until it is completed (public final void waitTillComplete() and its variants).

jlr52 commented 4 years ago

Apologies for the delay,

I fix all of the suggestions. Btw Async is available in rust 1.39 do we want to consider moving the support to that version to support Async for waitTillComplete , might not be worth it depending on the use case, I imagine udf and secondary index creation is usually not used on the fly and would be used in an one off script.

jhecking commented 4 years ago

Hi @jlr52,

I've merge all of the proposed changes back into your branch, and made some additional fixes as well. From my perspective, this PR is ready to merge now.

jlr52 commented 4 years ago

@jhecking Hey, super appreciated, I royally screwed up, life got in the way and the github alert email got buried in my inbox, and I totally forgot about this pull request, let me review it really fast, again super sorry, I glanced through your code review ask and they are all pretty reasonable ask, I just forgot about this......

jhecking commented 4 years ago

Hey, no worries! I was also busy with other things. But found some time today and thought would be good to wrap this up.

jlr52 commented 4 years ago

Everything looks really good honestly, alot of condition becomes more readable, some redundant code got deleted, also loved the optimization in Task module. Thanks for wrapping it up !!!!

question: should we create an issue about matches_override to get rid of the hack once we update rust version?

jhecking commented 4 years ago

question: should we create an issue about matches_override to get rid of the hack once we update rust version?

Sure, would be good to have that as a reminder.