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

Refactor flag parsing and runRule arguments for ruler #3993

Merged
merged 2 commits into from
Apr 16, 2021

Conversation

idoqo
Copy link
Contributor

@idoqo idoqo commented Mar 30, 2021

Signed-off-by: Michael Okoko okokomichaels@outlook.com

  • Change is not relevant to the end user.

Changes

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

It looks generally good, thanks! Will review more carefully later.

cmd/thanos/config.go Show resolved Hide resolved
bwplotka
bwplotka previously approved these changes Apr 6, 2021
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

LGTM, just let's ensure all flags are used/not missed. (will do that later)

@@ -428,6 +428,10 @@ Flags:
Prometheus relabel-config syntax. See format
details:
https://prometheus.io/docs/prometheus/latest/configuration/configuration/#relabel_config
--web.route-prefix="" Prefix for API and UI endpoints. This allows
Copy link
Member

Choose a reason for hiding this comment

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

This is great, but let's make sure this flag is handled and not registered and not used, by accident (:

@@ -269,73 +269,16 @@ Flags:
TLS CA to verify clients against. If no client
CA is specified, there is no client
verification on server side. (tls.NoClientCert)
--label=<name>="<value>" ...
Copy link
Member

Choose a reason for hiding this comment

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

Let's review manually here, but @idoqo can you add an issue on Thanos for the task to print flags in sorted order. (Let's make sure to provide the use case).

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>
Signed-off-by: Michael Okoko <okokomichaels@outlook.com>
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Nice! Now it's super easy to see what was added 🤗 Thanks to you @idoqo and @wiardvanrij for adding sorting!

@bwplotka bwplotka merged commit f4a5904 into thanos-io:main Apr 16, 2021
someshkoli pushed a commit to someshkoli/thanos that referenced this pull request Apr 21, 2021
* Refactor flag parsing and runRule arguments for ruler

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

* Regenerate sorted docs for rule and compactor

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>
Signed-off-by: someshkoli <kolisomesh27@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants