search

duneanalytics / dune-client

A framework for interacting with Dune Analytics' officially supported API service
Apache License 2.0
85 stars 22 forks source link

Refactor client; add tests for async usages #39

Closed eliseygusev closed 1 year ago

eliseygusev commented 1 year ago

Hello! Addressing your message https://github.com/cowprotocol/dune-client/pull/31#issuecomment-1316045313 (since I can not comment on closed PRs).

I couldn't really reproduce your issue, but I think, I know that one. So did some refactor to address the issues. Also, not sure where to put instructions on how to use the client, so added some tests with 3 different ways to use it (but also can add it anywhere else).

github-actions[bot] commented 1 year ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

bh2smith commented 1 year ago

Anyway, I think this looks pretty good for now. At least there are usable examples. If you wanted to sign the CLA (not sure why it has to be done twice, but...) I would merge this and publish an official release.

eliseygusev commented 1 year ago

Sorry, the CLA thing is probably result is me having different emails on my git... Hopefully is fixed now

bh2smith commented 1 year ago

Cool! Also, I have been reading up on this a bit and starting the like the idea of the context manager usage. However, we may need to add some hook to explicitly cancel executions upon exist from this environment.

bh2smith commented 1 year ago

All e2e tests passing on my end! Thanks again -- Feel free to improve the UX if you'd like!

bh2smith commented 1 year ago

I can't seem to get e2e tests to pass on any of python 3.7, 3.8, 3.9 or 3.10 in CI although it works perfectly fine locally for me... #41 ... any ideas here?