Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove or enhance AlgodResponseType #506

Open
tulustul opened this issue Aug 1, 2023 · 2 comments
Open

Remove or enhance AlgodResponseType #506

tulustul opened this issue Aug 1, 2023 · 2 comments
Labels
new-feature-request Feature request that needs triage

Comments

@tulustul
Copy link

tulustul commented Aug 1, 2023

Problem

Version 2.1.0 introduced AlgodResponseType that is returned by most of AlgodClient methods.
It's a union of dict and bytes meaning that I must cast the response type every time I use any algod method.

last_round = cast(dict, algod_client.status())["last-round"]
# instead of 
last_round = algod_client.status()["last-round"]

This complicates the code for no real benefit as the underlying dict is still not typed and the type checker has no idea what fields are inside the dict.

Now I have to add the pointless casting to my multiple repositories that use the algod.

It's a counter-productive way of typing things.

Solution

One of two things:

  1. Remove the return type completely on the algod methods.
  2. Make two versions of each method - one that returns bytes and one that returns a dict. A nice addition would be to have the dict fully typed.

Dependencies

N/A

Urgency

Low, nothing is broken but it prevents me from updating.

@tulustul tulustul added the new-feature-request Feature request that needs triage label Aug 1, 2023
@tzaffi
Copy link
Contributor

tzaffi commented Aug 10, 2023

Hi @tulustul. Thanks for opening the issue and your suggestions.

I'm in favor of suggestion (2) though it needs to be discussed further and prioritized. One possibility is that this ends up in the next major version of the SDK.

@jasonpaulos
Copy link
Member

I think having two versions for each method is the right solution. Having a parameter which changes the response type (as well as forwarding **kwargs to algod_request) adds a lot of complexity to these APIs.

However, a solution we could potentially adopt without a breaking change is using @overload to type multiple versions of these methods with different return types. However, this brings its own complexity and is more a bandaid than a deeper fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature-request Feature request that needs triage
Projects
None yet
Development

No branches or pull requests

3 participants