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

kvs: misc cleanup & refactoring #1383

merged 15 commits into from Mar 23, 2018


4 participants
Copy link

chu11 commented Mar 23, 2018

Prep work for #1341, this is random cleanups and minor refactoring.

chu11 added some commits Mar 9, 2018

modules/kvs: Fix comment errors
Fix misnamed function names in comments.
modules/kvs: Minor code cleanup
Place variable in smallest scope it is used.
modules/kvs: Minor kvs_util code cleanup
Use variable of separator instead of hard coding separator.
modules/kvs: Don't assume ENOMEM errno
When normalizing a key, do not automatically assume the error is
modules/kvs: Remove unused lookup functions
Remove lookup_get_path(), lookup_get_flags(), and lookup_get_cache().
modules/kvs: Rename kvsroot_mgr_t variables
To differentiate kvstxn_mgr_t from kvsroot_mgr_t variables,
use "krm" for kvsroot_mgr_t instead of "km".
modules/kvs: Remove lookup_set_aux_data()
Instead, pass aux data in lookup_create().  Slightly re-order arguments
in lookup_create() for consistency to other APIs internally.

Update callers and adjust unit tests appropriately.
modules/kvs: Cleanup/refactor lookup tests
Split check() function into check_value() and check_error(),
so that excessive parameters don't need to be passed in when
unnecessary.  In addition, remove unnecessary parameter to
modules/kvs: Remove rootdir from lookup RPC reply
Remove the "rootdir" in the RPC response to a lookup.   It is unused.
modules/kvs: Add convenience setroot to kvsroot
Add convenience functions kvsroot_setroot().

Use in main kvs file, add unit tests appropriately.
modules/kvs: Add convenience user check function
Add convenience function kvsroot_check_user().

Use in main kvs file, add unit tests appropriately.

This comment has been minimized.

Copy link

coveralls commented Mar 23, 2018

Coverage Status

Coverage increased (+0.02%) to 78.824% when pulling 1048ee3 on chu11:issue1341-part1 into 53e4898 on flux-framework:master.


This comment has been minimized.

Copy link

codecov-io commented Mar 23, 2018

Codecov Report

Merging #1383 into master will increase coverage by 0.01%.
The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           master    #1383      +/-   ##
+ Coverage   78.49%   78.51%   +0.01%     
  Files         162      162              
  Lines       29741    29724      -17     
- Hits        23345    23337       -8     
+ Misses       6396     6387       -9
Impacted Files Coverage Δ
src/modules/kvs/kvs_util.c 100% <100%> (ø) ⬆️
src/modules/kvs/lookup.c 78.6% <50%> (-1.21%) ⬇️
src/modules/kvs/kvsroot.c 72.41% <85.48%> (+4.09%) ⬆️
src/modules/kvs/kvs.c 64.68% <92.3%> (-0.36%) ⬇️
src/common/libflux/keepalive.c 86.66% <0%> (-6.67%) ⬇️
src/common/libflux/content.c 86.66% <0%> (-3.34%) ⬇️
src/common/libflux/mrpc.c 85.49% <0%> (-1.18%) ⬇️
src/common/libutil/base64.c 95.07% <0%> (-0.71%) ⬇️
src/common/libflux/message.c 81.48% <0%> (+0.23%) ⬆️
src/broker/overlay.c 74.76% <0%> (+0.31%) ⬆️
... and 4 more
modules/kvs: Remove invalid unit test
Do not call wait_get_usecount() after wait_runqueue() has destroyed
the wait structure.

Fixes #1384

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Mar 23, 2018

Added fix for #1384 and re-pushed.


This comment has been minimized.

Copy link

garlick commented Mar 23, 2018

Looks good! Thanks.

@garlick garlick merged commit 4223879 into flux-framework:master Mar 23, 2018

4 checks passed

codecov/patch 87.5% of diff hit (target 78.49%)
codecov/project 78.51% (+0.01%) compared to 53e4898
continuous-integration/travis-ci/pr The Travis CI build passed
coverage/coveralls Coverage increased (+0.02%) to 78.824%

@chu11 chu11 added this to To do in multi-user parallel jobs via automation Mar 26, 2018

@chu11 chu11 moved this from To do to In progress in multi-user parallel jobs Mar 27, 2018

@chu11 chu11 moved this from In progress to Done in multi-user parallel jobs Mar 27, 2018

@grondo grondo referenced this pull request May 10, 2018


0.9.0 Release #1479

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.