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

Fix monitoring for fedmsg-relay #433

Merged
merged 2 commits into from Jun 27, 2017

Conversation

Projects
None yet
3 participants
@jeremycline
Member

jeremycline commented Jun 26, 2017

This fixes an issue introduced in #395 that stopped the fedmsg relay
from starting a monitoring producer. This explicitly defines the
monitoring producer as the only producer so that any additional producer
entry points are not used.

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

@codecov-io

This comment has been minimized.

codecov-io commented Jun 26, 2017

Codecov Report

Merging #433 into develop will decrease coverage by 0.4%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #433      +/-   ##
===========================================
- Coverage    48.95%   48.54%   -0.41%     
===========================================
  Files           33       31       -2     
  Lines         2063     1963     -100     
  Branches       332      320      -12     
===========================================
- Hits          1010      953      -57     
+ Misses         951      913      -38     
+ Partials       102       97       -5
Impacted Files Coverage Δ
fedmsg/commands/relay.py 76.66% <100%> (+0.8%) ⬆️
fedmsg/tests/consumers/__init__.py
fedmsg/commands/__init__.py

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 7badf28...e8523ca. Read the comment docs.

@jeremycline jeremycline force-pushed the jeremycline:fix-monitoring-relays branch 2 times, most recently from 027b294 to d08cc0c Jun 27, 2017

@bowlofeggs

This comment has been minimized.

Member

bowlofeggs commented Jun 27, 2017

Tsk tsk, u wanna lower coverage bro?

@jeremycline

This comment has been minimized.

Member

jeremycline commented Jun 27, 2017

Whoa

@bowlofeggs

The removed test doesn't seem to be doing very many assertions, so I guess the lowered coverage number is metered by the fact that the test wasn't doing a good job of asserting the covered code.

'endpoints': {'relay_outbound': ['some-socket']},
}
self.expected_config = self.command.config.copy()
self.expected_config['zmq_subscribe_endpoints'] = 'circus,of,values'

This comment has been minimized.

@bowlofeggs

bowlofeggs Jun 27, 2017

Member

"Welcome to the circus of values "

Fix monitoring for fedmsg-relay
This fixes an issue introduced in #395 that stopped the fedmsg relay
from starting a monitoring producer. This explicitly defines the
monitoring producer as the only producer so that any additional producer
entry points are not used.

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

@jeremycline jeremycline force-pushed the jeremycline:fix-monitoring-relays branch from d08cc0c to d5383f5 Jun 27, 2017

Add unit tests for the fedmsg-relay command
Signed-off-by: Jeremy Cline <jeremy@jcline.org>

@jeremycline jeremycline force-pushed the jeremycline:fix-monitoring-relays branch from d5383f5 to e8523ca Jun 27, 2017

@jeremycline jeremycline merged commit c91bcc3 into fedora-infra:develop Jun 27, 2017

3 checks passed

codecov/patch 100% of diff hit (target 100%)
Details
codecov/project Absolute coverage decreased by -0.4% but relative coverage increased by +51.04% compared to 7badf28
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jeremycline jeremycline deleted the jeremycline:fix-monitoring-relays branch Jun 27, 2017

@jeremycline

This comment has been minimized.

Member

jeremycline commented Jun 27, 2017

This PR was cherry-picked onto the 0.18 branch as 0bb811b and 2eda4e0.

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