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: additional cleanup and refactoring #1273

Merged
merged 5 commits into from Nov 5, 2017

Conversation

chu11
Copy link
Member

@chu11 chu11 commented Nov 3, 2017

part three of issue #1264. This is the easy set of cleanups/refactoring. Don't think anything here will be considered too fascinating.

Only thing to note is the old kvs_util_json_has() function took a href_t, but treeobj_hash() takes a char * and int. It's b/c href_t is only defined for the KVS server right now. Considered removing href_t and generalizing it with char ref[NUM], but I elected not too.

Function now takes const void ** isntead of just void **.

Adjust callers appropriately.
Add new function to hash tree objects.  Add unit tests appropriately.
Replace with call to treeobj_hash().
With RFC11 changes, we always know the data in a treeobj val object
is a base64 encoded string.  Therefore, we do not need to call
kvs_util_json_encoded_size to get the encoded size.  We just
need to call strlen() on the string value.
With recent changes, kvs_util_json_dumps() and
kvs_util_json_encoded_size() are no longer used, so remove
them and associated unit tests.

Also remove now unnecessary header includes.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.007%) to 78.507% when pulling bebc347 on chu11:issue1264-part3 into 85353aa on flux-framework:master.

@codecov-io
Copy link

codecov-io commented Nov 4, 2017

Codecov Report

Merging #1273 into master will increase coverage by 0.01%.
The diff coverage is 95.23%.

@@            Coverage Diff             @@
##           master    #1273      +/-   ##
==========================================
+ Coverage   77.87%   77.88%   +0.01%     
==========================================
  Files         154      154              
  Lines       29138    29124      -14     
==========================================
- Hits        22691    22684       -7     
+ Misses       6447     6440       -7
Impacted Files Coverage Δ
src/modules/kvs/lookup.c 80.47% <ø> (ø) ⬆️
src/modules/kvs/kvs_util.c 100% <ø> (+12.24%) ⬆️
src/modules/kvs/commit.c 74.78% <100%> (+0.21%) ⬆️
src/common/libkvs/treeobj.c 86.2% <100%> (+0.66%) ⬆️
src/modules/kvs/cache.c 88.72% <100%> (ø) ⬆️
src/modules/kvs/kvs.c 62.62% <66.66%> (+0.25%) ⬆️
src/common/libflux/content.c 86.66% <0%> (-3.34%) ⬇️
src/common/libflux/rpc.c 92.56% <0%> (-0.83%) ⬇️
src/broker/overlay.c 73.88% <0%> (-0.64%) ⬇️
src/common/libflux/msg_handler.c 86.09% <0%> (-0.38%) ⬇️
... and 7 more

@garlick
Copy link
Member

garlick commented Nov 5, 2017

Looks great, thanks.

@garlick garlick merged commit d7300ec into flux-framework:master Nov 5, 2017
@grondo grondo mentioned this pull request May 10, 2018
@chu11 chu11 deleted the issue1264-part3 branch June 5, 2021 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants