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: Add src and dst namespace to kvs copy & move #1936

Merged
merged 6 commits into from Jan 22, 2019

Conversation

@chu11
Copy link
Contributor

commented Jan 19, 2019

first part of issue #1860. I've elected to make the new functions flux_kvs_commit_ns() and flux_kvs_lookup_ns() private. The API should have more of these functions (flux_kvs_fence_ns(), flux_kvs_lookupat_ns(), kvsdir, etc.) and tests, but I am leaving that work for another day.

@chu11

This comment has been minimized.

Copy link
Contributor Author

commented Jan 19, 2019

Hmmm, hit a failure in t2000-wreck-env

ERROR: t2000-wreck-env
======================
ok 1 - flux-wreck: setenv/getenv works
PASS: t2000-wreck-env.t 1 - flux-wreck: setenv/getenv works
ok 2 - flux-wreck: unsetenv works
PASS: t2000-wreck-env.t 2 - flux-wreck: unsetenv works
ok 3 - flux-wreck: setenv all
PASS: t2000-wreck-env.t 3 - flux-wreck: setenv all
ok 4 - wreck: global lwj.environ exported to jobs
PASS: t2000-wreck-env.t 4 - wreck: global lwj.environ exported to jobs
ok 5 - wreck: wreckrun exports environment vars not in global env
PASS: t2000-wreck-env.t 5 - wreck: wreckrun exports environment vars not in global env
ok 6 - wreck: wreckrun --skip-env works
PASS: t2000-wreck-env.t 6 - wreck: wreckrun --skip-env works
# passed all 6 test(s)
flux-start: 0 (pid 21034) Killed
1..6
ERROR: t2000-wreck-env.t - exited with status 137 (terminated by signal 9?)

it seems all tests passed, but I guess the broker took to long to shut down and it was killed?

@garlick

This comment has been minimized.

Copy link
Member

commented Jan 19, 2019

flux_kvs_lookupat_ns()

Shouldn't need that one since lookup starts from a specific hash, not from the root of a namespace.

@garlick

This comment has been minimized.

Copy link
Member

commented Jan 21, 2019

I went ahead and restarted that builder. The failure seems unlikely to be related to this PR.

@chu11

This comment has been minimized.

Copy link
Contributor Author

commented Jan 21, 2019

it seems all tests passed, but I guess the broker took to long to shut down and it was killed?

Doubt that this has anything to do with these changes. Was gonna restart the builder, but it seems someone beet me to it :-)

@chu11

This comment has been minimized.

Copy link
Contributor Author

commented Jan 21, 2019

I went ahead and restarted that builder. The failure seems unlikely to be related to this PR.

lol, what timing.

@garlick

This comment has been minimized.

Copy link
Member

commented Jan 21, 2019

I win!

chu11 added 6 commits Jan 7, 2019
Create new wrapper function flux_kvs_lookup_ns() to handle
lookup with a specific namespace.
In several locations, a bitwise OR (|) was accidentally
used instead of a logical OR (||).
Create new wrapper function flux_kvs_commit_ns() to handle
commit with a specific namespace.
Add headers to access flux_kvs_lookup_ns() and flux_kvs_commit_ns().
Have flux_kvs_copy() and flux_kvs_move() take namespace parameters
for the source & destination keys.  This allows copying and moving
between different namespaces without need for using the "ns:" prefix.
Add --src-namespace and --dst-namespace options to copy and move
commands.  This allows users to specify alternate namespaces without
using the ns prefix mechanism.
@chu11 chu11 force-pushed the chu11:issue1860_part1 branch from 78a70e8 to df8a1c3 Jan 22, 2019
@chu11

This comment has been minimized.

Copy link
Contributor Author

commented Jan 22, 2019

rebased

@codecov-io

This comment has been minimized.

Copy link

commented Jan 22, 2019

Codecov Report

Merging #1936 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1936      +/-   ##
==========================================
+ Coverage   80.05%   80.07%   +0.01%     
==========================================
  Files         195      195              
  Lines       35059    35078      +19     
==========================================
+ Hits        28065    28087      +22     
+ Misses       6994     6991       -3
Impacted Files Coverage Δ
src/common/libkvs/kvs_copy.c 75.9% <100%> (+3.68%) ⬆️
src/cmd/flux-kvs.c 83.25% <100%> (+0.07%) ⬆️
src/common/libkvs/kvs_commit.c 70% <100%> (-7.78%) ⬇️
src/common/libkvs/kvs_txn.c 73.26% <100%> (ø) ⬆️
src/common/libkvs/kvs_lookup.c 80.44% <100%> (-0.91%) ⬇️
src/broker/modservice.c 78.84% <0%> (-0.97%) ⬇️
src/common/libflux/message.c 81.64% <0%> (+0.12%) ⬆️
src/modules/connector-local/local.c 74.66% <0%> (+1.03%) ⬆️
@garlick

This comment has been minimized.

Copy link
Member

commented Jan 22, 2019

Thanks!

@garlick garlick merged commit 7f5e660 into flux-framework:master Jan 22, 2019
3 checks passed
3 checks passed
codecov/patch 100% of diff hit (target 80.05%)
Details
codecov/project 80.07% (+0.01%) compared to 2d647ec
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.