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

modules/aggregator: convert to jansson #1525

Merged
merged 2 commits into from May 17, 2018

Conversation

Projects
None yet
4 participants
@grondo
Copy link
Contributor

grondo commented May 17, 2018

Minor update to the aggregator module to convert to jansson. No other cleanup was attempted at this time (except to use pack/unpack where appropriate)

Still need to do a little more testing here, but wanted to go ahead and post the PR.

@grondo

This comment has been minimized.

Copy link
Contributor Author

grondo commented May 17, 2018

Oh one other change that fell out of this PR is the kvs entry created on rank 0 by the sink operation no longer exposes the ancillary data from the internal aggregator operation, i.e. the key, timeout members of the aggregate are not written to the kvs.

modules/aggregator: convert to jansson
Internally convert the aggregator module to jansson.

@grondo grondo force-pushed the grondo:aggregator-jansson branch from 55075da to dcb958f May 17, 2018

@coveralls

This comment has been minimized.

Copy link

coveralls commented May 17, 2018

Coverage Status

Coverage increased (+0.03%) to 78.895% when pulling dcb958f on grondo:aggregator-jansson into 03002f3 on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented May 17, 2018

Codecov Report

Merging #1525 into master will increase coverage by 0.03%.
The diff coverage is 82.85%.

@@            Coverage Diff            @@
##           master   #1525      +/-   ##
=========================================
+ Coverage   78.56%   78.6%   +0.03%     
=========================================
  Files         165     165              
  Lines       30801   30779      -22     
=========================================
- Hits        24200   24193       -7     
+ Misses       6601    6586      -15
Impacted Files Coverage Δ
src/modules/aggregator/aggregator.c 80.24% <82.85%> (+1.01%) ⬆️
src/common/libutil/blobref.c 97.22% <0%> (-1.39%) ⬇️
src/common/libkvs/kvs_watch.c 89.69% <0%> (-0.43%) ⬇️
src/common/libflux/handle.c 83.66% <0%> (-0.25%) ⬇️
src/common/libflux/message.c 81.05% <0%> (-0.24%) ⬇️
src/common/libutil/shortjson.h 91.17% <0%> (+0.15%) ⬆️
src/common/libutil/base64.c 95.77% <0%> (+0.7%) ⬆️
src/common/libflux/response.c 85.36% <0%> (+0.81%) ⬆️
src/broker/modservice.c 80.58% <0%> (+0.97%) ⬆️
src/broker/content-cache.c 74.73% <0%> (+1.29%) ⬆️
... and 1 more
@garlick

This comment has been minimized.

Copy link
Member

garlick commented May 17, 2018

Looks good! Happy to merge when you're ready.

@grondo

This comment has been minimized.

Copy link
Contributor Author

grondo commented May 17, 2018

Yeah, I think this one is straightforward enough to merge now.

@garlick garlick merged commit 2b1f9c8 into flux-framework:master May 17, 2018

4 checks passed

codecov/patch 82.85% of diff hit (target 78.56%)
Details
codecov/project 78.6% (+0.03%) compared to 03002f3
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.03%) to 78.895%
Details
@grondo

This comment has been minimized.

Copy link
Contributor Author

grondo commented May 17, 2018

Thanks!

@grondo grondo deleted the grondo:aggregator-jansson branch May 17, 2018

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