-
Notifications
You must be signed in to change notification settings - Fork 92
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
New Feature: Adds Data Directory Support #1012
Conversation
@shiqizng Please review to minimize merge conflicts with your branch. |
3eb33a1
to
3e23145
Compare
Yeah, it builds fine on my M1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requesting some small changes, and had some comments / questions.
README.md
Outdated
@@ -195,6 +195,7 @@ Settings can be provided from the command line, a configuration file, or an envi | |||
| default-balances-limit | | default-balances-limit | INDEXER_DEFAULT_BALANCES_LIMIT | | |||
| max-applications-limit | | max-applications-limit | INDEXER_MAX_APPLICATIONS_LIMIT | | |||
| default-applications-limit | | default-applications-limit | INDEXER_DEFAULT_APPLICATIONS_LIMIT | | |||
| data-dir | i | data-dir | INDEXER_DATA_DIR | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left this comment on Shiqi's PR also. Could we add a viper alias so that this is mapped to INDEXER_DATA
? That would allow us to skip the manual env variable checking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AlgoStephenAkiki does exporting INDEXER_DATA_DIR
work for you? after adding the alias, viper.RegisterAlias("data-dir", "data")
, only INDEXER_DATA
is working for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. Do we want both? Performing the alias I think removes the DATA_DIR variant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need both, lets update the docs to refer to INDEXER_DATA
. Nice catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI is failing. Otherwise, it looks good.
Codecov Report
@@ Coverage Diff @@
## develop #1012 +/- ##
===========================================
- Coverage 59.58% 59.52% -0.06%
===========================================
Files 45 45
Lines 8353 8353
===========================================
- Hits 4977 4972 -5
- Misses 2918 2922 +4
- Partials 458 459 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noticed a few more small things
Resolves #997 This PR does the following: - 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.
7c1760f
to
9c16950
Compare
* 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]>
* Local Ledger (#1011) * integrate block processor * Local Ledger Deployment (#1013) * add simple local ledger migration * add deleted opts * fast catchup (#1023) * add fast catchup * Localledger merge (#1036) * 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]> * recovery scenario (#1024) * handle ledger recovery scenario * refactor create genesis block (#1026) * refactor create genesis block * Adds Local Ledger Readme (#1035) * 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]> * update ledger file path and migration (#1042) * LocalLedger Refactoring + Catchpoint Service (#1049) 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 * build: merge develop into localledger/integration (#1062) * Ledger init status (#1058) * Generate an error if the catchpoint is not valid for initialization. (#1075) * Use main logger in handler and fetcher. (#1077) * Switch from fullNode catchup to catchpoint catchup service. (#1076) * Refactor daemon, add more tests (#1039) Refactors daemon cmd into separate, testable pieces. * Merge develop into localledger/integration (#1083) * Misc Local Ledger cleanup (#1086) * Update processor/blockprocessor/initialize.go Co-authored-by: Zeph Grunschlag <[email protected]> * commit * fix function call args * RFC-0001: Rfc 0001 impl (#1069) Adds an Exporter interface and a noop exporter implementation with factory methods for construction * Fix test errors * Add/fix tests * Add postgresql_exporter tests * Update config loading * Change BlockExportData to pointers * Move and rename ExportData * Add Empty func to BlockData * Add comment Co-authored-by: shiqizng <[email protected]> Co-authored-by: [email protected] <[email protected]> Co-authored-by: erer1243 <[email protected]> Co-authored-by: AlgoStephenAkiki <[email protected]> Co-authored-by: Will Winder <[email protected]> Co-authored-by: Zeph Grunschlag <[email protected]>
Resolves #997
This PR does the following:
to be options of only the daemon sub-command
that defer's are handled instead of being ignored.
issues errors if equivalent command line arguments are supplied.
configuration files and the data directory.