Thanos: compactor: Simplify CLI flags

Created on 13 Mar 2020  路  7Comments  路  Source: thanos-io/thanos

Currently, we have --wait and --wait-interval (recently added) to run compactor in repeated intervals. However, API could be simplified, by just keeping one. See the discussion.
Before introducing any breaking changes, let's collect some feedback about the usage.

We should remove one of them before stabilizing the API.
Adding this issue here keep track of it.

compact feature request / improvement good first issue help wanted stale

All 7 comments

This issue/PR has been automatically marked as stale because it has not had recent activity. Please comment on status otherwise the issue will be closed in a week. Thank you for your contributions.

Is it still valid?

@kakkoyun, @bwplotka I was looking at continuing on with fixing #2248 and started to look at compact component first so naturally I came across this.

I was wondering if dropping the wait flag and setting wait-interval to a default of 0s and then add a check like

if interval != time.Second * 0 { // rununtil }

wouldn't achieve the same result as having both flags with wait defaulting to false currently?
Maybe this is confusing to the user though, wdyt?

@PhilipGough We had the same discussion with @bwplotka, when we first add the flag https://github.com/thanos-io/thanos/pull/2265.

Since you also raised the same point again, I think we should get rid of the additional flag.
wait-interval also a bit misleading, run-interval maybe?
Or if we can have an optional value for a flag, so that we can have --wait with the default interval and wait=Xs with the specified interval.

All in all, we can simplify and remove the extra flag.

Hello 馃憢 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 馃
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

Hello 馃憢 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 馃
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

Closing for now as promised, let us know if you need this to be reopened! 馃

Was this page helpful?
0 / 5 - 0 ratings

Related issues

abursavich picture abursavich  路  4Comments

jdfalk picture jdfalk  路  3Comments

hedeesaa picture hedeesaa  路  3Comments

rmrf picture rmrf  路  4Comments

barryib picture barryib  路  4Comments