Skip to content

Docs: Update API docs, remove unused codegen configs#4896

Merged
algorandskiy merged 2 commits intoalgorand:masterfrom
Eric-Warehime:api-tag-doccs
Dec 13, 2022
Merged

Docs: Update API docs, remove unused codegen configs#4896
algorandskiy merged 2 commits intoalgorand:masterfrom
Eric-Warehime:api-tag-doccs

Conversation

@Eric-Warehime
Copy link
Copy Markdown
Contributor

Summary

Updates Readme in the algod api directory to include a short summary of the tag usage for the V2 APIs.

Also deleted some of the initial codegen configs that were created during the transition to separating the APIs. make clean && make returns no diffs in the generated code which I used to confirm they were indeed unused.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 12, 2022

Codecov Report

Merging #4896 (2ba4e94) into master (fb95de1) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4896      +/-   ##
==========================================
- Coverage   54.06%   54.04%   -0.02%     
==========================================
  Files         427      427              
  Lines       53520    53520              
==========================================
- Hits        28935    28927       -8     
- Misses      22316    22325       +9     
+ Partials     2269     2268       -1     
Impacted Files Coverage Δ
ledger/tracker.go 74.26% <0.00%> (-3.80%) ⬇️
ledger/blockqueue.go 85.63% <0.00%> (-2.88%) ⬇️
catchup/service.go 69.80% <0.00%> (-0.25%) ⬇️
ledger/testing/randomAccounts.go 56.61% <0.00%> (ø)
ledger/acctupdates.go 69.24% <0.00%> (+0.24%) ⬆️
ledger/acctonline.go 78.29% <0.00%> (+0.51%) ⬆️
ledger/voters.go 73.13% <0.00%> (+4.47%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Copy Markdown

@letonchanh letonchanh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Copy Markdown
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a good improvement to me, but I did want to point out an issue I noticed that deserves some thought.

endpoints based on the use case for the node.

Each API in `algod.oas2.json`, except for some pre-existing `common` APIs, should have two tags.
1. Either `public` or `private`. This controls the type of authentication used by the API--the `public` APIs use the
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the goal is to have the data endpoints use admin authentication, but also be visible by the SDKs, something needs to change, since they will ignore private endpoints.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up switching the new data endpoints to public for this reason.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Adding a link to #4900 in case anyone else is following this

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants