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

flux-kvs: misc fixes #2136

Merged
merged 5 commits into from Apr 19, 2019

Conversation

@chu11
Copy link
Contributor

commented Apr 19, 2019

Started just wanting to fix issue #2086, and found two other issues along the way.

For issue #2129 (fix issues unlinking symlinks), the unlink_safety_check() function was a little tricky to begin with, so I re-worked the whole function do the check based on the lookup
of a TREEOBJ, hopefully minimizing the number of KVS lookups that have to be done to do an unlink_safety_check().

chu11 added 5 commits Apr 17, 2019
Namespace specified by the user was not passed to categorize_key().

Add test for coverage.

Fixes #2132
In one location, the un-normalized key is output instead of the
normalized key.  For consistency, always output the normalized
key.
Fix output of flux-kvs ls to be more like ls(1).  Most notably:

- when -d or -F are specified, output link name
- when -d or -F are not specified, follow link if it points to a dir

Add new tests accordingly.

Fixes #2086
Fix corner cases in kvs unlink in regards to links.

If the link pointed to an invalid target, namespace, or
had an infinite cycle, the unlink_safety_check() function
would fail and not allow the user to remove the link.

Instead, unlink should always allow links to be removed.
Re-work unlink_saftety_check() to ensure links are always
removable.

Add regression tests.

Fixes #2129
@chu11 chu11 force-pushed the chu11:issue2086_2129_2132 branch from a79de82 to 4833835 Apr 19, 2019
@codecov-io

This comment has been minimized.

Copy link

commented Apr 19, 2019

Codecov Report

Merging #2136 into master will increase coverage by 0.01%.
The diff coverage is 82.6%.

@@            Coverage Diff            @@
##           master   #2136      +/-   ##
=========================================
+ Coverage   80.39%   80.4%   +0.01%     
=========================================
  Files         201     201              
  Lines       31710   31742      +32     
=========================================
+ Hits        25493   25522      +29     
- Misses       6217    6220       +3
Impacted Files Coverage Δ
src/cmd/flux-kvs.c 81.72% <82.6%> (-0.49%) ⬇️
src/modules/connector-local/local.c 73.62% <0%> (-1.04%) ⬇️
src/common/libflux/message.c 81.64% <0%> (+0.12%) ⬆️
src/common/libutil/aux.c 94.44% <0%> (+3.7%) ⬆️
src/modules/job-ingest/worker.c 72.72% <0%> (+7.69%) ⬆️
@garlick

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

This seems straighforward. Ready to merge?

@chu11

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2019

yup

@garlick garlick merged commit a1ebb9c into flux-framework:master Apr 19, 2019
4 checks passed
4 checks passed
Mergify — Summary 1 potential rule
Details
codecov/patch 82.6% of diff hit (target 80.39%)
Details
codecov/project 80.4% (+0.01%) compared to 9a8e6ef
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
Projects
None yet
3 participants
You can’t perform that action at this time.