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

Split algod and indexer generated API client models #59

Open
jasonpaulos opened this issue Feb 2, 2023 · 1 comment
Open

Split algod and indexer generated API client models #59

jasonpaulos opened this issue Feb 2, 2023 · 1 comment

Comments

@jasonpaulos
Copy link
Contributor

jasonpaulos commented Feb 2, 2023

Background

This SDK provides clients for algod and indexer. The code for these clients are generated by https://github.com/algorand/generator. The generator makes two passes, one for algod, and one for indexer.

Problem

The generator outputs both algod and indexer models to the client/v2/common/models folder. I believe the motivation behind this was that the algod and indexer response types are so similar, it seemed desirable to use the same types for them. And since indexer responses tended to have a few more fields, the indexer models are generated second and override the algod ones.

The problem is that we frequently introduce new fields to algod, and indexer is not always quick to adopt them. A motivating example is the account min balance field, which algod has but indexer to this day does not yet support: algorand/indexer#808 (this feature is more complicated to implement in indexer, so it's not something a quick PR can easily solve).

Because of this discrepancy in fields, this SDK does not support the min balance account field at all, even though algod provides it (#272).

Proposed Solution

Since it's not always possible/easy for us to maintain perfect consistency between algod and indexer responses, I think the best solution is to separate the response definitions for them in this SDK. In theory that's as simple as modifying the generator to output the models to different packages. In practice, we would need a major version bump, since all the response types would be located somewhere else.

And there is a possibility that clients would need to do further work to accept this change, since now algod and indexer would produce differently defined structs. But I don't think this is very likely. For instance, the JS SDK separates algod and indexer definitions, and we don't anticipate problems to arise because of that.

@algoanne algoanne transferred this issue from algorand/go-algorand-sdk Apr 18, 2023
@winder
Copy link
Contributor

winder commented May 4, 2023

With the recent trend to think about each SDK individually rather than all of them as a whole, I think this issue should be copied to each SDK and addressed in the context of a major version update if/when one of those is planned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants