feat: deprecate ui enabled #1227
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
Codecov Report
@@ Coverage Diff @@
## main #1227 +/- ##
==========================================
+ Coverage 79.89% 80.00% +0.11%
==========================================
Files 38 39 +1
Lines 2805 2831 +26
==========================================
+ Hits 2241 2265 +24
- Misses 460 461 +1
- Partials 104 105 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
markphelps
commented
Dec 15, 2022
| @@ -45,10 +44,14 @@ type Config struct { | |||
| Database DatabaseConfig `json:"db,omitempty" mapstructure:"db"` | |||
| Meta MetaConfig `json:"meta,omitempty" mapstructure:"meta"` | |||
| Authentication AuthenticationConfig `json:"authentication,omitempty" mapstructure:"authentication"` | |||
| Warnings []string `json:"warnings,omitempty"` | |||
There was a problem hiding this comment.
This does however have the side effect that Warnings will no longer be returned in the JSON output when hitting /meta/config endpoint..
Two thoughts here:
- We could just remove it since it isn't documented anywhere
- Or we could figure out another way to add these
Warningsback in before marshalling the config as JSON
There was a problem hiding this comment.
I would lean on dropping them from that endpoint and letting warnings be logs only.
We probably could get away with it. Those warnings are meant to be ephemeral over time as you fix your config.
I wouldn't expect anything to be depending on their context for something behavioural.
markphelps
changed the title
markphelps
commented
Dec 15, 2022
| @@ -52,15 +52,15 @@ func (c *CacheConfig) setDefaults(v *viper.Viper) { | |||
| func (c *CacheConfig) deprecations(v *viper.Viper) []deprecation { | |||
| var deprecations []deprecation | |||
|
|
|||
| if v.GetBool("cache.memory.enabled") { | |||
There was a problem hiding this comment.
this was only being evaluated if cache.memory.enabled was set to true
markphelps
commented
Dec 15, 2022
| deprecations = append(deprecations, deprecation{ | ||
|
|
||
| option: "cache.memory.enabled", | ||
| additionalMessage: deprecatedMsgMemoryEnabled, | ||
| }) | ||
| } | ||
|
|
||
| if v.IsSet("cache.memory.expiration") { | ||
| if v.InConfig("cache.memory.expiration") { | ||
| deprecations = append(deprecations, deprecation{ |
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.
Re: FLI-125
Refactors config parsing to separate out
Warningsfrom theConfigitself, making it easier to write tests against config parsingDeprecates
ui.enabledas in the future we will have 2 versions of Flipt: