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 fixes & cleanup #1805

Merged
merged 6 commits into from Nov 6, 2018

Conversation

Projects
None yet
4 participants
@chu11
Copy link
Contributor

chu11 commented Nov 6, 2018

misc fixes / cleanup while working on #1651.

  • return ENOTSUP instead of ENOENT when a namespace is removed, is more consistent with other error returns.
  • use waitfile script in tests, b/c I didn't know it existed when I wrote a lot of the watch tests eons ago.
  • remove now unnecessary waits and buffering in tests
  • remove an unnecessary flag clear
  • fix some cut & paste typos

chu11 added some commits Nov 1, 2018

t/kvs: Remove wait_watch_file in kvs tests
Use waitfile.lua script to accomplish same purpose.
cmd/flux-kvs: Fix command line help
Fix command line help instructions for flux kvs get watch, which
were cut and pasted incorrectly.
t/kvs: Remove kvs wait_watch helper functions
Remove wait_watch_put, wait_watch_put_namespace, and wait_watch_empty,
which are no longer needed b/c racyness in the tests was removed
by other means a long time ago.
t/kvs: Remove stdbuf calls when unnecessary
Remove majority of stdbuf calls when buffering kvs calls.  Most of
them are now unnecessary that we use waitfile.
common/libkvs: Remove FLUX_KVS_WATCH clear
The clear did not accomplish anything.
modules/kvs-watch: Change namespace remove errno
When a namespace is removed, return ENOTSUP instead of ENOENT
to the user.  This is more consistent with the usage of ENOTSUP
in the main kvs to indicate when a namespace does not exist.  In
addition, it allows the user to differentiate between a namespace
no longer existing and a key no longer existing.
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 6, 2018

Codecov Report

Merging #1805 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1805      +/-   ##
==========================================
- Coverage   79.62%   79.62%   -0.01%     
==========================================
  Files         186      186              
  Lines       34563    34562       -1     
==========================================
- Hits        27522    27519       -3     
- Misses       7041     7043       +2
Impacted Files Coverage Δ
src/cmd/flux-kvs.c 81.8% <ø> (ø) ⬆️
src/common/libkvs/kvs_getroot.c 89.47% <100%> (-0.14%) ⬇️
src/modules/kvs-watch/kvs-watch.c 75.13% <100%> (ø) ⬆️
src/common/libflux/response.c 79.62% <0%> (-1.24%) ⬇️
@garlick

This comment has been minimized.

Copy link
Member

garlick commented Nov 6, 2018

LGTM! Ready to merge?

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Nov 6, 2018

I say push the button here so we can get #1798 then #1800 & #1787 subsequently rebased and merged!

@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Nov 6, 2018

yup

@garlick garlick merged commit 5e4ff81 into flux-framework:master Nov 6, 2018

3 checks passed

codecov/patch 100% of diff hit (target 79.62%)
Details
codecov/project Absolute coverage decreased by -<.01% but relative coverage increased by +20.37% compared to 7394541
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
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.