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

Add a fedmsg-check command #416

Merged
merged 1 commit into from Jun 9, 2017

Conversation

Projects
None yet
3 participants
@jeremycline
Member

jeremycline commented Jun 7, 2017

This command makes use of the moksha monitor socket to determine the
health of consumers and producers.

fixes #415

Signed-off-by: Jeremy Cline jeremy@jcline.org

timeout (int): The length of time in seconds to wait for a ZMQ message
on the monitor socket.
consumer (tuple): A list of consumer names to check the status of.
producer (tuple): A list of producer names to check the status of.

This comment has been minimized.

@bowlofeggs

bowlofeggs Jun 8, 2017

Member

I recommend against documenting the args like this on click commands - if the user types --help on this command, this entire docblock will be displayed and won't be particularly helpful or useful to the user. It will also give duplicate info since the help will also already tell them about the flags.

See fedora-infra/bodhi#1457

This comment has been minimized.

@jeremycline

jeremycline Jun 9, 2017

Member

Good call, thanks!

if missing:
raise click.ClickException('Some consumers and/or producers are missing!')
elif uninitialized:
raise click.ClickException('Some consumers and/or producers are uninitialized!')

This comment has been minimized.

@bowlofeggs

bowlofeggs Jun 8, 2017

Member

Might you want this to be an if instead of an elif? I.e., if some are missing and others are uninitialized, it'd be nice to report both to the user instead of one or the other. Of course, that breaks the final else, so you probably need to wrap the above into an overall if missing or unitialized and then do inner ifs…

This comment has been minimized.

@jeremycline

jeremycline Jun 9, 2017

Member

It actually does report all of them in the for loop above, this is mostly about explaining the reason it exited non-zero.

@@ -1,3 +1,4 @@
click

This comment has been minimized.

@bowlofeggs

bowlofeggs Jun 8, 2017

Member

Don't regular requirements get installed in addition to test requirements? If so, I'd think you don't need to put click here since it's part of the usual requirements…

This comment has been minimized.

@jeremycline

jeremycline Jun 9, 2017

Member

This is what I was doing before I learned how to tell tox to install the package with various extras, I'll remove it :)

@jeremycline jeremycline force-pushed the jeremycline:fedmsg-check branch 2 times, most recently from 38b7f04 to c4d492d Jun 9, 2017

@codecov-io

This comment has been minimized.

codecov-io commented Jun 9, 2017

Codecov Report

Merging #416 into develop will increase coverage by 1.28%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #416      +/-   ##
===========================================
+ Coverage    45.93%   47.21%   +1.28%     
===========================================
  Files           32       33       +1     
  Lines         2018     2067      +49     
  Branches       324      333       +9     
===========================================
+ Hits           927      976      +49     
  Misses         996      996              
  Partials        95       95
Impacted Files Coverage Δ
fedmsg/commands/check.py 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97db0c6...c35eff1. Read the comment docs.

Add a fedmsg-check command
This command makes use of the moksha monitor socket to determine the
health of consumers and producers.

fixes #415

Signed-off-by: Jeremy Cline <jeremy@jcline.org>

@jeremycline jeremycline force-pushed the jeremycline:fedmsg-check branch from c4d492d to c35eff1 Jun 9, 2017

@jeremycline jeremycline merged commit 5899512 into fedora-infra:develop Jun 9, 2017

3 checks passed

codecov/patch 100% of diff hit (target 45.93%)
Details
codecov/project 47.21% (+1.28%) compared to 97db0c6
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jeremycline jeremycline deleted the jeremycline:fedmsg-check branch Jun 9, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment