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

Provide "from_env" constructor in "DuneClient" class

Open PaulRBerg opened this issue 10 months ago • 8 comments

Problem

PyLance throws type errors when the Dune client is obtained by using the from_env constructor because the type returned is BaseDuneClient, which doesn't contain the methods available in QueryAPI:

  • https://github.com/duneanalytics/DuneQueryRepo/issues/11
  • https://github.com/duneanalytics/DuneQueryRepo/issues/12
  • https://github.com/duneanalytics/DuneQueryRepo/issues/9
  • https://github.com/duneanalytics/DuneQueryRepo/issues/8

Solution

I'm not a Python expert but I think that the solution is to override from_env in the DuneClient class so that it returns the type like this:

def from_env(cls) -> DuneClient:

Workaround

In the meantime, the workaround is to use the typing package like so:

dune = cast(DuneClient, DuneClient.from_env())

PaulRBerg avatar Apr 12 '25 17:04 PaulRBerg

So, I think that the general problem is the inheritance pattern used here is whack. Never should have been allowed in the first place. One workaround that might suffice for this version (without breaking the interface to do things properly) might be adding your suggestion into the Dune Client class declaration like so:

    @classmethod
    def from_env(cls) -> DuneClient:
        return cast(DuneClient, BaseDuneClient.from_env())

Then, at least, you wouldn't have to perform the cast all over your other code.

However the real solution here is to follow a more appropriate design pattern (composition over inheritance) - similar to what we have done in the Node Client: https://github.com/duneanalytics/ts-dune-client/blob/039a74136b1d49bcdac701e5660dd4a1c73d3fee/src/api/client.ts#L36-L55

Sorry about the trouble this has caused and thanks for reporting.

bh2smith avatar Apr 14 '25 09:04 bh2smith

Thanks for the quick response and guidance @bh2smith

Side note - is anyone from the Dune team watching the DuneQueryRepo?

I have never received any response to my issues/PRs there.

PaulRBerg avatar Apr 14 '25 12:04 PaulRBerg

That's a great question @PaulRBerg I had not heard of this repo until it was referenced here today. Might not hurt to tag one of the contributors there... like @andrewhong5297

bh2smith avatar Apr 14 '25 12:04 bh2smith

I took a quick look at what that repo is for and it seems a bit like these scripts might be used in CI to maintain a query registry. If that is your usecase, I would recommend taking a look at the dune-update gihub action

  • Repo: https://github.com/bh2smith/dune-update/
  • Marketplace: https://github.com/marketplace/actions/dune-query-updater
  • Demo Project Workflow: https://github.com/bh2smith/demo-ts-dune-client/blob/main/.github/workflows/ci.yaml
  • Real Project Workflow: https://github.com/cowprotocol/dune-queries/blob/main/.github/workflows/ci.yaml

This updater uses the NodeJS Dune Client rather than the python one (making it more compatible for GitHub Actions)

bh2smith avatar Apr 14 '25 12:04 bh2smith

Thanks @bh2smith I didn't even know about that GitHub Action — it might be worth documenting it in your Dune docs:

https://docs.dune.com/api-reference/quickstart/queries-eg

PaulRBerg avatar Apr 14 '25 12:04 PaulRBerg

it might be worth documenting it in your Dune doc

Yes, it certainly would. However as of now this is still an unofficial independent project that I have personally maintained. Its still pre-v1 and I haven't gotten much feedback. So it may not quite qualify to belong in the official docs. If you wind up using it, please do feel free to leave some feedback. Would be great to tag a v1 release.

bh2smith avatar Apr 14 '25 12:04 bh2smith

(A little tongue-in-cheek) well, not sure how many teams can afford the $400/monthly plan for Dune Plus, so putting in the official docs should be fine because the paywall prevents the software from being used by many folks

and if someone pays $400 for that, they probably know what they are doing

PaulRBerg avatar Apr 14 '25 13:04 PaulRBerg

Anyway, let me know if you try it out and have any feature requests.

bh2smith avatar Apr 22 '25 13:04 bh2smith

This constructor is deprecated now, and if you just use the DuneClient() one it will read your env and you shouldn't have any issues. from_env will be removed in an upcoming release.

mewwts avatar Sep 29 '25 11:09 mewwts