New Feature: Lookup Boxes by app-id + Name and Search Boxes by app-id #1117
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* integrate block processor
* add simple local ledger migration
* add fast catchup
* return empty lists from fetchApplications and fetchAppLocalStates (#1010) * Update model to converge with algod (#1005) * New Feature: Adds Data Directory Support (#1012) - Updates the go-algorand submodule hash to point to rel/beta - Moves the cpu profiling file, pid file and indexer configuration file to be options of only the daemon sub-command - Changes os.Exit() to be a panic with a special handler. This is so that defer's are handled instead of being ignored. - Detects auto-loading configuration files in the data directory and issues errors if equivalent command line arguments are supplied. - Updates the README with instructions on how to use the auto-loading configuration files and the data directory. * Update mockery version Co-authored-by: erer1243 <[email protected]> Co-authored-by: AlgoStephenAkiki <[email protected]>
* handle ledger recovery scenario
* refactor create genesis block
* Adds Local Ledger Readme Resolves #4109 Starts Readme docs * Update docs/LocalLedger.md Co-authored-by: Will Winder <[email protected]> * Update docs/LocalLedger.md Co-authored-by: Will Winder <[email protected]> * Update docs/LocalLedger.md Co-authored-by: Will Winder <[email protected]> * Removed troubleshooting section Co-authored-by: Will Winder <[email protected]>
Part 1
cleanup genesis file access.
put node catchup into a function that can be swapped out with the catchup service.
pass the indexer logger into the block processor.
move open ledger into a util function, and move the initial state util function into a new ledger util file.
add initial catchupservice implementation.
move ledger init from daemon.go to constructor.
Merge multiple read genesis functions.
Part 2
Merge local_ledger migration package into blockprocessor.
Rename Migration to Initialize
Use logger in catchup service catchup
Part 3
Update submodule and use NewWrappedLogger.
Make util.CreateInitState private
…lgorand commit ...
…recognized sig type
…tzaffi/box-ingest
tzaffi
commented
Sep 13, 2022
tzaffi
commented
Sep 13, 2022
tzaffi
commented
Sep 13, 2022
tzaffi
commented
Sep 13, 2022
tzaffi
commented
Sep 14, 2022
| @@ -1613,16 +1711,38 @@ | |||
| } | |||
| } | |||
| }, | |||
| "ParticipationUpdates": { | |||
There was a problem hiding this comment.
The ParticipationUpdates has been moved further below to starting line 1834 and following the OnCompletion section
winder
reviewed
Sep 14, 2022
winder
reviewed
Sep 14, 2022
winder
reviewed
Sep 14, 2022
winder
reviewed
Sep 14, 2022
Co-authored-by: Will Winder <[email protected]>
Co-authored-by: Will Winder <[email protected]>
Co-authored-by: Will Winder <[email protected]>
…tzaffi/box-search
michaeldiamant
approved these changes
Sep 14, 2022
winder
reviewed
Sep 14, 2022
Comment on lines
+2327
to
+2330
| orderKind := "ASC" | ||
| if queryOpts.Ascending != nil && !*queryOpts.Ascending { | ||
| orderKind = "DESC" | ||
| } |
There was a problem hiding this comment.
Is this feature user visible? On some tables the indexes are optimized for one vs the other, so there could be performance implications here.
There was a problem hiding this comment.
No. It's not visible. Can we continue the conversation on #1168 ?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related PR's
Summary
Exposing box information for querying indexer. In particular:
v2/applications/:application-id/boxv2/applications/:application-id/boxes/v2/accounts/:account-id(account info):total-box-bytestotal-boxesTODO / Questions
.../boxesworking as expected? ANSWER: yes/v2/accounts/:account-idfixtures_test to show that we're exposing the new box fields even for non-app related accountsMaxBoxResultsPerAppErrora la MaxAPIResourcesPerAccountError ? NO - this is unecessary because the built-in limit facilities in indexer should already guard against unexpected response size when querying for boxes. An adjacent question might be: should we take boxes into account as part of theMaxAPIResourcesPerAccountcontrols during the querying of account information? The question here is "no" as well because detailed box information isn't currently included in theGetAccounts()query and therefore doesn't impact the limit.boxes/search parameternextrequires ab64:rather than providing what was received fornext-tokenin the previous response. This violates indexer usage patterns. TODO: adhere to the typical non-modification pattern fornext-tokenin indexer. This is now done, but controvertially,b64:is being removed altogether. We can adhere to the opaquness property of next while still allowing arbitrary encoding if we simply return the next token with ab64:prefix. This is under active discussion. CONCLUSION: we're returning the b64 prefix in next token, and accepting arbitrary encoding in the same way that box names are handled in the/box?name=...endpointORDER BY(i.e. theorderparam) is not exposed in theboxes/endpoint, or a consensus in a going forward policy (e.g. issue creation, or change request) CONCLUSION: hard coded ASC ordering for boxes in/boxes/endpointfixtures_test.goexist? Or is there sufficient end-to-end testing coverage inhandlers_e2e_test.goto obviate this new test? Answer: YES, it should exist because it has utility as we can see: per this commit I'm conforming to the same response code and error as with other indexer treatments. However, I made a commitment to:box/andboxes/endpoints. Should this continue to exist as a required query parameter, or should instead become a path parameter? UPDATE: a consensus has been reached, and that is to keep the treatment of the endpoints as they currently are in the PR.Test Plan
CI