Skip to content
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

mimirtool: deprecate rule-files flag #7756

Merged
merged 7 commits into from
Apr 4, 2024

Conversation

armandgrillet
Copy link
Contributor

@armandgrillet armandgrillet commented Mar 31, 2024

What this PR does

A few mimirtool commands accepted a rule-files flag: rules diff, rules sync, rules prepare, rules lint, and rules check. However, all these commands also accept a list of rule files as argument. Therefore, the flag is a duplicate which is inconsistent.

This flag originated in 9b206ce. Back then, the list of rule files was only an argument for one command: rules load, and that command never got a rule-files flag. The rule files argument was added later to more commands, in https://github.com/grafana/cortex-tools/pull/125/files, but the existing rule-files flag was not removed.

This will be a breaking change for folks relying on mimirtool in scripts, for now the flag is only deprecated.

Which issue(s) this PR fixes or relates to

Fixes #6747

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

The flag is unnecessary since it is an argument in all updated commands.
@CLAassistant
Copy link

CLAassistant commented Mar 31, 2024

CLA assistant check
All committers have signed the CLA.

@colega
Copy link
Contributor

colega commented Apr 1, 2024

This is technically a breaking change for folks relying on mimirtool in scripts.

We should issue a deprecation notice before deleting the flag (and it's functionality): https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md#deprecated-features

And then we should keep it for 2 versions more, otherwise we might break people's CI workflows.

@armandgrillet armandgrillet changed the title mimirtool: remove rule-files flag mimirtool: deprecate rule-files flag Apr 1, 2024
@armandgrillet
Copy link
Contributor Author

@colega Thanks for the review, PR updated to only deprecate the flags.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com>
Copy link
Contributor

@colega colega left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

@colega colega merged commit a2a2ffa into main Apr 4, 2024
29 checks passed
@colega colega deleted the armandgrillet/remove-rule-files-flag branch April 4, 2024 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mimirtool sync flags ambiguous
3 participants