client-rust icon indicating copy to clipboard operation
client-rust copied to clipboard

Published 20 hours ago • verified publisher icon tikv

Add sync API for raw client

Open xuhui-lu opened this issue 4 years ago • 14 comments

The raw part work of https://github.com/tikv/client-rust/issues/289

xuhui-lu avatar Jun 13 '21 09:06 xuhui-lu

Thanks! I'm wondering if it is better to have a separate sync RawClient, instead of mixing sync and async API in the same struct. How do you like it?

Also, could you please add appropriate example(s) in the README and/or under the example directory? It would best help the new users who are the expected users of sync API.

ekexium avatar Jun 15 '21 03:06 ekexium

Thanks! I'm wondering if it is better to have a separate sync RawClient, instead of mixing sync and async API in the same struct. How do you like it?

Also, could you please add appropriate example(s) in the README and/or under the example directory? It would best help the new users who are the expected users of sync API.

I think it will be like a SyncRawClient which wraps the RawClient insides. And sure, I will add some examples into README doc.

xuhui-lu avatar Jun 15 '21 06:06 xuhui-lu

LGTM. Fix the fmt check please.

ekexium avatar Jul 05 '21 10:07 ekexium

hey @ekexium , could you please help take a look at it? THX!

xuhui-lu avatar Jul 14 '21 04:07 xuhui-lu

@xuhui-lu Could you take a look at these checks and make sure they pass? In your local environment, you could use make check and make unit-test to run these checks. image

ekexium avatar Jul 14 '21 06:07 ekexium

@xuhui-lu Could you take a look at these checks and make sure they pass? In your local environment, you could use make check and make unit-test to run these checks. image

it is weird, it all passed before I merge master into it. Let me take a look.

xuhui-lu avatar Jul 14 '21 06:07 xuhui-lu

@xuhui-lu Oh right. A recent PR just changed the API

ekexium avatar Jul 14 '21 06:07 ekexium

@xuhui-lu Oh right. A recent PR just changed the API

cool, I have merge that change into my PR. It seems that the master branch's CI failed also.

xuhui-lu avatar Jul 14 '21 07:07 xuhui-lu

The master branch is fine. The failed action is going to be removed. #309

ekexium avatar Jul 14 '21 07:07 ekexium

#309

hey @ekexium, when will the fix patch be merged? And then I could solve these test failures.

xuhui-lu avatar Jul 21 '21 05:07 xuhui-lu

The failed action is about deploying the doc and is only executed on the master branch, so it shouldn't block merging this PR. You could fix the tests first.

ekexium avatar Jul 21 '21 05:07 ekexium

The failed action is about deploying the doc and is only executed on the master branch, so it shouldn't block merging this PR. You could fix the tests first.

sure, unit test fixed.

xuhui-lu avatar Jul 21 '21 06:07 xuhui-lu

LGTM. Could you sign off your last commit please?

ekexium avatar Aug 09 '21 10:08 ekexium

I recently found that I need a synchronous interface with timeout in my development work. Will this PR continue to be developed? Can I continue this work?

Smityz avatar Jun 14 '23 03:06 Smityz