feat(types): Deprecate the DBBackend variable in favor of new app-db-backend config entry #11188
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
…nction. Create a NewDB function to replace them.
…or setting it. Update the simulation setup to use that instead of the compile-time DBBackend variable.
…ure what to do about the db backend in that case though.
…penDB and uppdating calls to that to provide the db type to use.
… type if an empty string is provided there.
…'s uses, there doesn't seem to be any desire to change it, and there's no easy way to communicate it.
… that is being returned.
dwedul-figure
commented
Feb 14, 2022
dwedul-figure
changed the title
dwedul-figure
commented
Feb 14, 2022
dwedul-figure
changed the title
4 tasks
tac0turtle
approved these changes
Feb 14, 2022
dwedul-figure
changed the title
# Conflicts: # CHANGELOG.md
# Conflicts: # .github/workflows/codeql-analysis.yml
# Conflicts: # CHANGELOG.md
# Conflicts: # Makefile
|
@alexanderbez and @robert-zaremba do you think you'll be able to have another look at this? |
…that helps with address-in-use and non-empty directory errors.
Comment on lines
+20
to
+22
| - uses: actions/setup-go@v3 | ||
| with: | ||
| go-version: 1.17 |
There was a problem hiding this comment.
This duplicates the actions/setup-go@v3 step below?
There was a problem hiding this comment.
Yup. I'd guess it was a bad merge on my part (added both by me in my branch and then later in main).
I created #11420 to remove one of the entries.
Of course. Maps are, by definition, un-ordered. This is fundamental knowledge.
Any code which relies on deterministic ordering of map iteration is buggy, absolutely. That doesn't mean it's appropriate to prevent map iteration altogether. It would be like preventing the use of negative indices in slice expressions, because that can produce out-of-bounds errors. Or preventing dereferencing of pointers, because that can produce nil pointer exceptions. The cure, here, is worse than the disease. |
larry0x
pushed a commit
to larry0x/cosmos-sdk
that referenced
this pull request
May 22, 2023
…backend config entry (cosmos#11188) * [10948]: Add changelog entry. * [10948]: Deprecate the types.DBBackend variable and the NewLevelDB function. Create a NewDB function to replace them. * [10948]: Add a DBBackend string to the simulation config and a flag for setting it. Update the simulation setup to use that instead of the compile-time DBBackend variable. * [10948]: Update the mock app creator to use the NewDB function. Not sure what to do about the db backend in that case though. * [10948]: Update changelog to reflect new db-backend field name. * [10948]: Use the tendermint db-backend type for the snapshot db. * [10948]: Update the last use of NewLevelDB by adding a parameter to openDB and uppdating calls to that to provide the db type to use. * [10948]: Upddate the NewDB function to also have a default db backend type if an empty string is provided there. * [10948]: Remove the new TODO in mock.NewApp. After looking through it's uses, there doesn't seem to be any desire to change it, and there's no easy way to communicate it. * [10948]: Enhance the NewDB defer function to also add info to any err that is being returned. * [10948]: Add some unit tests for NewDB. * [10948]: Lint fixes. * [10948]: Add a changelog entry to the deprecated section. * [10948]: Update the makefile to no longer set the types.DBBackend value. * [10948]: Use memdb for the mock app instead of goleveldb. I know it was a goleveldb before, but for a mock app, a memdb feels like a better choice (assuming 'mock' and 'mem' mean what I assume they mean). * [10948]: Fix the store benchmark tests (had some index-out-of-range issues). * [10948]: Fix cachekv store bench test calling iter.Key() before checking iter.Valid(). * [10948]: Remove the panic recovery from types.NewDB since dbm.NewDB returns an error now (it didn't originally, when NewLevelDB was first written). * [10948]: Add changlog entry indicationg an API breaking change due to the DBBackend change. * [10948]: Get rid of the types.NewDB function in favor of just using the tm-db version of it. * [10948]: Fix Update the codeql-analysis github action to use go v1.17. * [10948]: Add config file option for the app db backend type. * [10948]: Adjust the comment on the app-db-backend config entry to clarify fallback behavior. * [10948]: Add a default of GoLevelDBBackend to GetAppDBBackend. The old DBBackend variable defaulted to that, and some unit tests assume that behavior still exists. * [10948]: Add the missing quotes around the app-db-backend value. * [10948]: Small tweak to the changelog's deprecated entry. * Add the go version declaration back into the codeql-analysis github action. * [10948]: Update new use of openDB. * [10948]: Put a brief delay after closing the test network. Hopefully that helps with address-in-use and non-empty directory errors. Co-authored-by: Marko <[email protected]>
4 tasks
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.
Description
Closes: #10948
This PR deprecates the
types.DBBackendvariable andtypes.NewLevelDBfunction. It replaces them with a newapp.tomlconfig entry:app-db-backendand thedbm.NewDBfunction from github.com/tendermint/tm-db. Uses ofNewLevelDBwere replaced with calls toNewDBand the db backend value comes from the tendermint configdb-backendentry. The simapp stuff doesn't have the tendermint or app configs, so aDBBackendsim config field anddb-backendflag was added in order to know the db backend value to use for them.Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
addedN/A!to the type prefix if API or client breaking changefollowed the guidelines for building modulesN/ACHANGELOG.mdupdated the relevant documentation or specificationN/AReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change