-
Notifications
You must be signed in to change notification settings - Fork 535
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
fix: remove unused properties from the WAL config file #3911
Conversation
CHANGELOG.md
Outdated
@@ -50,7 +50,7 @@ | |||
* [BUGFIX] Improved handling of complete blocks in localblocks processor after enabling flusing [#3805](https://github.com/grafana/tempo/pull/3805) (@mapno) | |||
* [BUGFIX] Handle out of boundaries spans kinds [#3861](https://github.com/grafana/tempo/pull/3861) (@javiermolinar) | |||
* [BUGFIX] Maintain previous tenant blocklist on tenant errors [#3860](https://github.com/grafana/tempo/pull/3860) (@zalegrala) | |||
|
|||
* [BUGFIX] Remove unused properties from the WAL configuration [#3911](https://github.com/grafana/tempo/pull/3911) (@javiermolianr) |
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.
we need to list this as a breaking change. we do that by typing **Breaking Change**
on the changelog line like this:
Line 3 in d0f0a5f
* [CHANGE] **BREAKING CHANGE** Remove `autocomplete_filtering_enabled` feature flag [#3729](https://github.com/grafana/tempo/pull/3729) (@mapno) |
that will help us find and note the removal of these config optoins for our users when we cut 2.6
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.
Done, I didn't think this was a breaking change because the configuration was not honored ever :P
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 is a good point, but we are removing config options that previously were parsed. It might break someone's config I suppose.
@@ -597,8 +595,6 @@ storage: | |||
queue_depth: 20000 | |||
wal: | |||
path: /var/tempo/wal | |||
completedfilepath: /var/tempo/wal/completed |
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.
nice. i'm impressed you found and updated the manifest :)
What this PR does:
It removes the properties
completedfilepath
andblocksfilepath
from the configuration of the WAL. The former one is not needed anymore and setting the latter one is a bit pointless since it can be just hanging fromfile path
Which issue(s) this PR fixes:
Fixes #3902
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]