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

Rule checker's CLI usability #618

Closed
fabxc opened this Issue Mar 31, 2015 · 8 comments

Comments

Projects
None yet
3 participants
@fabxc
Copy link
Member

fabxc commented Mar 31, 2015

  • $ rule_checker -h shows flags intended for Prometheus that were defined in imported packages. An explicit flag set for the rule checker should fix that.
  • The obvious input is a file - so having a -rule-file="" flag is inconvenient (and documented wrongly as -ruleFile on the website) . It should simply use the first non-flag argument, i.e. $ rule_checker my/fancy.rules (with -rule-file="" for backwards-compatibility).
  • The file argument should be a glob.
  • (The rule checker prints standard-formatted output (just that there hasn't been a never-ending discussion what the correct standard-format is, yet), which is close to what #21 asks for. It would just need a -w flag that overwrites the input file. Comments have to be preserved for that, however.)
@matthiasr

This comment has been minimized.

Copy link
Contributor

matthiasr commented Mar 31, 2015

I don't think it should glob in the tool – it should just consume all non-option arguments as input filenames (and stdin if there are none). This matches standard unix tool behaviour and combines best with shell globbing or other tools.

A second level of glob expansion would make it practically impossible to pass in filenames that contain special characters, and open a lot of avenues for things to go wrong. In interactive invocation, globbing is the shell's job.

@fabxc

This comment has been minimized.

Copy link
Member Author

fabxc commented Mar 31, 2015

You are right - wasn't thinking there, thanks.

@fabxc fabxc changed the title Rule checker's CLI interface usability Rule checker's CLI usability Mar 31, 2015

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Mar 31, 2015

👍 And if no files are specified, it should read from stdin, like any other self-respecting Unix tool.

@fabxc

This comment has been minimized.

Copy link
Member Author

fabxc commented Mar 31, 2015

Great.
Overwriting probably needs some discussion on what the one-and-only correct syntax is.

I could do the rest now.
Currently the rule checker panics on everything - thus, it exits with 255, which is also documented. Can we break backwards-compatibility here? I would rather go for 2 for non-existent files/directories (consistent with flag package) and 1 for actual processing errors.

I doubt there is anyone checking for anything other than non-zero.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Mar 31, 2015

@fabxc We have no idea if anyone is doing that, but as long as it's in the CHANGELOG.md for the next release and we're in version 0.x, it's totally fine to change.

Regarding #21 I think that's very low priority for now. It'll be a nice thing to have long-term though.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Apr 15, 2015

This can be closed now, right?

@fabxc

This comment has been minimized.

Copy link
Member Author

fabxc commented Apr 15, 2015

Yes, the remains are covered by the old ticket.

@juliusv juliusv closed this Apr 15, 2015

simonpasquier pushed a commit to simonpasquier/prometheus that referenced this issue Oct 12, 2017

@lock

This comment has been minimized.

Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.