-
Notifications
You must be signed in to change notification settings - Fork 556
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
feat: migration system for the config file #2792
Merged
Merged
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
f090c3f
tbruyelle
previously approved these changes
Sep 26, 2022
tbruyelle
previously approved these changes
Sep 29, 2022
This is required to make sure the values read from YAML files can be merged and serialized to other formats like JSON.
tbruyelle
previously approved these changes
Oct 3, 2022
aljo242
previously approved these changes
Oct 3, 2022
tbruyelle
approved these changes
Oct 4, 2022
Jchicode
pushed a commit
to Jchicode/cli
that referenced
this pull request
Aug 9, 2023
* Separate the struct definitions of the chainconfig from the parser * Move the struct Config into v0 package * Add and implement the interface Config * Parse the yaml based on the version * Move the filed Build into the common struct * Create the interface for validator * Change the parameter for parse * Add the conversion * Replace v0 with v1 after parsing * Fix the getInit for v1 * Update the parser and the validator * Replace v0 with v1 in env * Change the template * Fixes the issue with the GetHost interface * Add the method to increment the port number * Move FaucetHost to the common package * Remove the default() in config * Type the Version * Refactor the function Parse * Add the test to verify the Parse with migration * Clean up the interface of Config * Update the subcommands for ignite chain * Add tests on the ConfigYaml an some comments * Add the struct defining Gentx * Remove the getters in BaseConfig * Fix the function ConvertLatest * Remove all getters * refactor createValidatorFromConfig * Refactor the common pkg * Change FillValidatorsDefaults and add tests * a * chore: remove empty file * refactor: simplify file names * refactor: improve tests layout * refactor: add `config` package to `chainconfig` This change resolves a cyclic import issue * refactor: rename `Config` interface to `Converter` * chore: change testdata to give BaseConfig precedence * refactor: rename `Config.ConfigVersion` to `Config.Version` The interface's `Version()` method was also renamed to `GetVersion()`. * chore: code cleanup for consistency * chore: remove unused `BaseConfig.AccountByName` method * refactor: move `ListAccounts()` into `BaseConfig` * refactor: rename `ErrCouldntLocateConfig` to `ErrConfigNotFound` * chore: fix docstrings and broken test * refactor: improve v0 to v1 conversion * feat: add `xnet` package * refactor: improve `v1` config implementation - Simplify `Validator` to remove many getter methods - Removed some global variables - Improve unit tests - Add more unit tests - Remove unit tests that were not useful * refactor: remove global and rename var `Migration` to `Versions` * refactor: add default config function to `config` Also added `SetDefaults` to be able to initialize configs during conversion to default values. * refactor: change `ConvertLatest` to return `*v1.Config` This function must always return the latest config. * refactor: move `ErrConfigNotFound` to `errors.go` Also changed `UnsupportedVersionError` to contain the version number instead of an error message. * fix: change v1 `ConvertNext` to assign 1 to the version * test: add `TestConvertV0ToV1` to `v0` convert tests Also removed the `TestConvertNext`. * test: change `v0` testdata to only return a v0 config with values * fix: various fixes related to the refactor * fix: parsing and migration now work using interfaces * refactor: change migration cmd verifier The verifier now stops the command when the config migration is not made. * fix: change config validation to validate "validators" * chore: change `Seek` calls to use a constant argument * refactor: change to use standard reader/writers instead of seeker * test: add test for `MigrateLatest` * test: move testdata config files inside its corresponding version The config files and `GetConfig()` functions for each version not live inside the `v0` and `v1` package. * refactor: rewrite `Parse()` tests * fix: changed `v1` set defaults for don't ovewrite existing values * chore: add more values to testdata config files * test: add `TestParseWithUnknownVersion` test * chore: remove `GetConfigV0` from `v0` testdata Test must use `v0testdata.GetConfig()` instead. * test: rewrite `v0` clone test * fix: change imports from "ignite-hq" to "ignite" * fix: change integration tests to work with the new config * chore: update changelog * chore: fix integration test argument for `IsAppServed` * test: fix broken integration tests * chore(pkg/xnet): add `MustIncreasePortBy` to simplify tests * refactor: use struct to manage config's server addresses Map type causes a lot of trouble and extra casting and checks when the Cosmos SDK or Tendermint config values for the server addresses are read or modified so instead of that this changeset uses a struct. We can't use Cosmos SDK ot Tendermint config structs because if any of there configs gets updated it could break the migrations. The changeset also fixes the default address values that were encoded to the YAML config. These values no longer appear if they are the default ones. * chore: minor corrections * chore: remove unneeded `SetDefaults` call * fix: change init chain to support merging YAML generated maps The contributed implementation was removing some fields which was not correct; Now those config fields are saved during init. It seems that the map fields were being removed to avoid an issue with maps containing interface{} key types instead of string. This change fixes it by saving all the config values and by adding support for YAML generated maps thought a transformer implementation. * refactor: remove merge transformer in favor of a custom map type The mergo transformer didn't work properly so it was replaced with a custom map type that implements the yaml.Unmarshaller interface to convert the map keys to strings right after they are decoded. * refactor: change chain's init to update app config files only once The blockchain app config file were updated twice, once by the stargate plugin and then again by the chain init. This changeset updated the app's config only once. The stargate plugin is the one that updates them. The changeset also solves the issue where come of the addresses that must have the protocol prefix were ovewritten with an address without prefix because the host config value was removed (ignite#2470), so now addresses are defined in the app and config sections. * test: fix broken integration test * refactor: trigger config migration verifier from a pre run handler * refactor: change config migration cmd handler to use `cliui` package * refactor: add a type alias for the latest config version * ci: fix typo * chore: rename variables to follow standard * refactor: change `addGitChangesVerifier` to a cmd pre run handler * refactor: add confirmation before migrating config in unclean state A confirmation is shown now before migrating the confir file to the latest version if the app repository has uncommitted changes. * fix: change config clonning to copy using mergo package * chore: replace deprecated call to ReadFile * chore: fix formatting * tests: change `IsAppServed` to receive the API address * fix: change `pkg/yaml` to convert keys in slice of map This is required to make sure the values read from YAML files can be merged and serialized to other formats like JSON. * tests: fix broken network publish integration test * chore: add `gitChangesConfirmPreRunHandler` to `ts-client` cmd * test: fix broken TS client bank module integration test Co-authored-by: vinbrucelu <[email protected]> Co-authored-by: İlker G. Öztürk <[email protected]> Co-authored-by: Alex Johnson <[email protected]>
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.
Closes #2470