Skip to content

Commit

Permalink
cmd/flux-kvs: add [--json|--raw] options to "get"
Browse files Browse the repository at this point in the history
Problem:  flux-get put presumes values should be interpreted as JSON,
but the KVS no longer requires this.

Add type options mirroring those added to the "put" subcommand
to allow the user to choose how values are interpreted.

If no options, value is interpreted as a NULL terminated string
and written to stdout with a newline.

If --raw, value is interpreted as raw data and is written to
stdout without any formatting.

If --json, value is interpreted as encoded JSON.  If the value is an
object or array, or any scalar type but string, it is displayed
in its encoded form.  If the value is a JSON string, quotes are
removed, which mimics the old default behavior of flux-kvs get
expected by many sharness tests.

Add --json to flux kvs get where used in various sharness tests.

Fixes flux-framework#1159
  • Loading branch information
garlick committed Oct 26, 2017
1 parent f7e3412 commit 2892d92
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 62 deletions.
40 changes: 34 additions & 6 deletions src/cmd/flux-kvs.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ static void dump_kvs_dir (const flux_kvsdir_t *dir, bool Ropt, bool dopt);

#define min(a,b) ((a)<(b)?(a):(b))

static struct optparse_option get_opts[] = {
{ .name = "json", .key = 'j', .has_arg = 0,
.usage = "Interpret value(s) as encoded JSON",
},
{ .name = "raw", .key = 'r', .has_arg = 0,
.usage = "Interpret value(s) as raw data",
},
OPTPARSE_TABLE_END
};

static struct optparse_option put_opts[] = {
{ .name = "json", .key = 'j', .has_arg = 0,
.usage = "Store value(s) as encoded JSON",
Expand Down Expand Up @@ -130,11 +140,11 @@ static struct optparse_option unlink_opts[] = {

static struct optparse_subcommand subcommands[] = {
{ "get",
"key [key...]",
"[-j|-r] key [key...]",
"Get value stored under key",
cmd_get,
0,
NULL
get_opts
},
{ "put",
"[-j|-r] key=value [key=value...]",
Expand Down Expand Up @@ -349,7 +359,7 @@ static void output_key_json_str (const char *key,
int cmd_get (optparse_t *p, int argc, char **argv)
{
flux_t *h = (flux_t *)optparse_get_data (p, "flux_handle");
const char *key, *json_str;
const char *key;
flux_future_t *f;
int optindex, i;

Expand All @@ -360,10 +370,28 @@ int cmd_get (optparse_t *p, int argc, char **argv)
}
for (i = optindex; i < argc; i++) {
key = argv[i];
if (!(f = flux_kvs_lookup (h, 0, key))
|| flux_kvs_lookup_get (f, &json_str) < 0)
if (!(f = flux_kvs_lookup (h, 0, key)))
log_err_exit ("%s", key);
output_key_json_str (NULL, json_str, key);
if (optparse_hasopt (p, "json")) {
const char *json_str;
if (flux_kvs_lookup_get (f, &json_str) < 0)
log_err_exit ("%s", key);
output_key_json_str (NULL, json_str, key);
}
else if (optparse_hasopt (p, "raw")) {
const void *data;
int len;
if (flux_kvs_lookup_get_raw (f, &data, &len) < 0)
log_err_exit ("%s", key);
if (write_all (STDOUT_FILENO, data, len) < 0)
log_err_exit ("%s", key);
}
else {
const char *value;
if (flux_kvs_lookup_get (f, &value) < 0)
log_err_exit ("%s", key);
printf ("%s\n", value);
}
flux_future_destroy (f);
}
return (0);
Expand Down
4 changes: 2 additions & 2 deletions t/issues/t0441-kvs-put-get.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ TEST=issue441

flux kvs put --json ${TEST}.x=foo

flux kvs get ${TEST}.x.y && test $? -eq 1
flux kvs get --json ${TEST}.x.y && test $? -eq 1

flux kvs get ${TEST}.x # fails if broker died
flux kvs get --json ${TEST}.x # fails if broker died
4 changes: 2 additions & 2 deletions t/issues/t0821-kvs-segfault.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@

TEST=issue0821
flux kvs put --json ${TEST}="xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
flux kvs get ${TEST}.x && test $? -eq 1
flux kvs get ${TEST} # fails if broker died
flux kvs get --json ${TEST}.x && test $? -eq 1
flux kvs get --json ${TEST} # fails if broker died
62 changes: 31 additions & 31 deletions t/t1000-kvs.t
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ SUBDIR1=test.a.b.d
SUBDIR2=test.a.b.e

test_kvs_key() {
flux kvs get "$1" >output
flux kvs get --json "$1" >output
echo "$2" >expected
test_cmp expected output
}
Expand Down Expand Up @@ -139,43 +139,43 @@ EOF
'
test_expect_success 'kvs: unlink works' '
flux kvs unlink $KEY.integer &&
test_must_fail flux kvs get $KEY.integer
test_must_fail flux kvs get --json $KEY.integer
'
test_expect_success 'kvs: unlink works' '
flux kvs unlink $KEY.double &&
test_must_fail flux kvs get $KEY.double
test_must_fail flux kvs get --json $KEY.double
'
test_expect_success 'kvs: unlink works' '
flux kvs unlink $KEY.string &&
test_must_fail flux kvs get $KEY.string
test_must_fail flux kvs get --json $KEY.string
'
test_expect_success 'kvs: unlink works' '
flux kvs unlink $KEY.emptystring &&
test_must_fail flux kvs get $KEY.emptystring
test_must_fail flux kvs get --json $KEY.emptystring
'
test_expect_success 'kvs: unlink works' '
flux kvs unlink $KEY.jsonnull &&
test_must_fail flux kvs get $KEY.jsonnull
test_must_fail flux kvs get --json $KEY.jsonnull
'
test_expect_success 'kvs: unlink works' '
flux kvs unlink $KEY.strnull &&
test_must_fail flux kvs get $KEY.strnull
test_must_fail flux kvs get --json $KEY.strnull
'
test_expect_success 'kvs: unlink works' '
flux kvs unlink $KEY.booleantrue &&
test_must_fail flux kvs get $KEY.booleantrue
test_must_fail flux kvs get --json $KEY.booleantrue
'
test_expect_success 'kvs: unlink works' '
flux kvs unlink $KEY.booleanfalse &&
test_must_fail flux kvs get $KEY.booleanfalse
test_must_fail flux kvs get --json $KEY.booleanfalse
'
test_expect_success 'kvs: unlink works' '
flux kvs unlink $KEY.array &&
test_must_fail flux kvs get $KEY.array
test_must_fail flux kvs get --json $KEY.array
'
test_expect_success 'kvs: unlink works' '
flux kvs unlink $KEY.object &&
test_must_fail flux kvs get $KEY.object
test_must_fail flux kvs get --json $KEY.object
'
test_expect_success 'kvs: unlink dir works' '
flux kvs unlink $SUBDIR1 &&
Expand All @@ -194,7 +194,7 @@ test_expect_success 'kvs: put (multiple)' '
flux kvs put --json $KEY.a=42 $KEY.b=3.14 $KEY.c=foo $KEY.d=true $KEY.e="[1,3,5]" $KEY.f="{\"a\":42}"
'
test_expect_success 'kvs: get (multiple)' '
flux kvs get $KEY.a $KEY.b $KEY.c $KEY.d $KEY.e $KEY.f >output &&
flux kvs get --json $KEY.a $KEY.b $KEY.c $KEY.d $KEY.e $KEY.f >output &&
cat >expected <<EOF &&
42
3.140000
Expand Down Expand Up @@ -231,12 +231,12 @@ EOF
'
test_expect_success 'kvs: unlink (multiple)' '
flux kvs unlink $KEY.a $KEY.b $KEY.c $KEY.d $KEY.e $KEY.f &&
test_must_fail flux kvs get $KEY.a &&
test_must_fail flux kvs get $KEY.b &&
test_must_fail flux kvs get $KEY.c &&
test_must_fail flux kvs get $KEY.d &&
test_must_fail flux kvs get $KEY.e &&
test_must_fail flux kvs get $KEY.f
test_must_fail flux kvs get --json $KEY.a &&
test_must_fail flux kvs get --json $KEY.b &&
test_must_fail flux kvs get --json $KEY.c &&
test_must_fail flux kvs get --json $KEY.d &&
test_must_fail flux kvs get --json $KEY.e &&
test_must_fail flux kvs get --json $KEY.f
'
test_expect_success 'kvs: unlink -R works' '
flux kvs unlink -R $DIR &&
Expand Down Expand Up @@ -400,11 +400,11 @@ test_expect_success 'kvs: ls key. fails if key does not exist' '
#

test_expect_success 'kvs: get a nonexistent key' '
test_must_fail flux kvs get NOT.A.KEY
test_must_fail flux kvs get --json NOT.A.KEY
'
test_expect_success 'kvs: try to retrieve a directory as key should fail' '
flux kvs mkdir $DIR.a.b.c &&
test_must_fail flux kvs get $DIR
test_must_fail flux kvs get --json $DIR
'

#
Expand Down Expand Up @@ -510,7 +510,7 @@ test_expect_success 'kvs: link works' '
TARGET=$DIR.target &&
flux kvs put --json $TARGET=\"foo\" &&
flux kvs link $TARGET $DIR.link &&
OUTPUT=$(flux kvs get $DIR.link) &&
OUTPUT=$(flux kvs get --json $DIR.link) &&
test "$OUTPUT" = "foo"
'
test_expect_success 'kvs: readlink works' '
Expand Down Expand Up @@ -548,13 +548,13 @@ test_expect_success 'kvs: link: path resolution when intermediate component is a
flux kvs unlink -Rf $DIR &&
flux kvs put --json $DIR.a.b.c=42 &&
flux kvs link $DIR.a.b $DIR.Z.Y &&
OUTPUT=$(flux kvs get $DIR.Z.Y.c) &&
OUTPUT=$(flux kvs get --json $DIR.Z.Y.c) &&
test "$OUTPUT" = "42"
'
test_expect_success 'kvs: link: path resolution with intermediate link and nonexistent key' '
flux kvs unlink -Rf $DIR &&
flux kvs link $DIR.a.b $DIR.Z.Y &&
test_must_fail flux kvs get $DIR.Z.Y
test_must_fail flux kvs get --json $DIR.Z.Y
'
test_expect_success 'kvs: link: intermediate link points to another link' '
flux kvs unlink -Rf $DIR &&
Expand Down Expand Up @@ -657,14 +657,14 @@ test_expect_success 'kvs: link: error on link depth' '
flux kvs link $DIR.i $DIR.j &&
flux kvs link $DIR.j $DIR.k &&
flux kvs link $DIR.k $DIR.l &&
test_must_fail flux kvs get $DIR.l
test_must_fail flux kvs get --json $DIR.l
'

test_expect_success 'kvs: link: error on link depth, loop' '
flux kvs unlink -Rf $DIR &&
flux kvs link $DIR.link1 $DIR.link2 &&
flux kvs link $DIR.link2 $DIR.link1 &&
test_must_fail flux kvs get $DIR.link1
test_must_fail flux kvs get --json $DIR.link1
'

#
Expand All @@ -674,8 +674,8 @@ test_expect_success 'kvs: copy works' '
flux kvs unlink -Rf $DIR &&
flux kvs put --json $DIR.src=\"foo\" &&
flux kvs copy $DIR.src $DIR.dest &&
OUTPUT1=$(flux kvs get $DIR.src) &&
OUTPUT2=$(flux kvs get $DIR.dest) &&
OUTPUT1=$(flux kvs get --json $DIR.src) &&
OUTPUT2=$(flux kvs get --json $DIR.dest) &&
test "$OUTPUT1" = "foo" &&
test "$OUTPUT2" = "foo"
'
Expand All @@ -684,8 +684,8 @@ test_expect_success 'kvs: move works' '
flux kvs unlink -Rf $DIR &&
flux kvs put --json $DIR.src=\"foo\" &&
flux kvs move $DIR.src $DIR.dest &&
test_must_fail flux kvs get $DIR.src &&
OUTPUT=$(flux kvs get $DIR.dest) &&
test_must_fail flux kvs get --json $DIR.src &&
OUTPUT=$(flux kvs get --json $DIR.dest) &&
test "$OUTPUT" = "foo"
'

Expand Down Expand Up @@ -726,7 +726,7 @@ test_expect_success NO_CHAIN_LINT 'kvs: version and wait' '

wait_watch_put() {
i=0
while [ "$(flux kvs get $1 2> /dev/null)" != "$2" ] && [ $i -lt 50 ]
while [ "$(flux kvs get --json $1 2> /dev/null)" != "$2" ] && [ $i -lt 50 ]
do
sleep 0.1
i=$((i + 1))
Expand All @@ -740,7 +740,7 @@ wait_watch_put() {

wait_watch_empty() {
i=0
while flux kvs get $1 2> /dev/null && [ $i -lt 50 ]
while flux kvs get --json $1 2> /dev/null && [ $i -lt 50 ]
do
sleep 0.1
i=$((i + 1))
Expand Down
10 changes: 5 additions & 5 deletions t/t1002-kvs-extra.t
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ KEY=test.a.b.c
#
#
test_kvs_key() {
flux kvs get "$1" >output
flux kvs get --json "$1" >output
echo "$2" >expected
test_cmp output expected
#if ! test "$OUTPUT" = "$2"; then
Expand Down Expand Up @@ -201,7 +201,7 @@ test_expect_success 'kvs: put-treeobj: clobbers destination' '
flux kvs put --json $TEST.a=42 &&
${KVSBASIC} get-treeobj . >snapshot2 &&
${KVSBASIC} put-treeobj $TEST.a="`cat snapshot2`" &&
! flux kvs get $TEST.a &&
! flux kvs get --json $TEST.a &&
flux kvs dir $TEST.a
'

Expand Down Expand Up @@ -294,7 +294,7 @@ test_expect_success 'kvs: valref that points to content store data can be read'
flux kvs unlink -Rf $TEST &&
echo "$largeval" | flux content store &&
${KVSBASIC} put-treeobj $TEST.largeval2="{\"data\":[\"${largevalhash}\"],\"type\":\"valref\",\"ver\":1}" &&
flux kvs get $TEST.largeval2 | grep $largeval
flux kvs get --json $TEST.largeval2 | grep $largeval
'

test_expect_success 'kvs: valref that points to zero size content store data can be read' '
Expand Down Expand Up @@ -400,13 +400,13 @@ test_expect_success 'kvs: store 3x4 directory tree using kvsdir_put functions' '
test_expect_success 'kvs: put on rank 0, exists on all ranks' '
flux kvs put --json $TEST.xxx=99 &&
VERS=$(flux kvs version) &&
flux exec sh -c "flux kvs wait ${VERS} && flux kvs get $TEST.xxx"
flux exec sh -c "flux kvs wait ${VERS} && flux kvs get --json $TEST.xxx"
'

test_expect_success 'kvs: unlink on rank 0, does not exist all ranks' '
flux kvs unlink -Rf $TEST.xxx &&
VERS=$(flux kvs version) &&
flux exec sh -c "flux kvs wait ${VERS} && ! flux kvs get $TEST.xxx"
flux exec sh -c "flux kvs wait ${VERS} && ! flux kvs get --json $TEST.xxx"
'

# commit test
Expand Down
10 changes: 5 additions & 5 deletions t/t2000-wreck.t
Original file line number Diff line number Diff line change
Expand Up @@ -182,25 +182,25 @@ test_expect_success 'wreckrun: -N without -n works' '
test_expect_success 'wreckrun: -N without -n sets ntasks in kvs' '
flux wreckrun -l -N${SIZE} /bin/true &&
LWJ=$(last_job_path) &&
n=$(flux kvs get ${LWJ}.ntasks) &&
n=$(flux kvs get --json ${LWJ}.ntasks) &&
test "$n" = "${SIZE}"
'
test_expect_success 'wreckrun: -n without -N sets nnnodes in kvs' '
flux wreckrun -l -n${SIZE} /bin/true &&
LWJ=$(last_job_path) &&
n=$(flux kvs get ${LWJ}.nnodes) &&
n=$(flux kvs get --json ${LWJ}.nnodes) &&
test "$n" = "${SIZE}"
'
test_expect_success 'wreckrun: -t1 -N${SIZE} sets ntasks in kvs' '
flux wreckrun -l -t1 -N${SIZE} /bin/true &&
LWJ=$(last_job_path) &&
n=$(flux kvs get ${LWJ}.ntasks) &&
n=$(flux kvs get --json ${LWJ}.ntasks) &&
test "$n" = "${SIZE}"
'
test_expect_success 'wreckrun: -t1 -n${SIZE} sets nnodes in kvs' '
flux wreckrun -l -t1 -n${SIZE} /bin/true &&
LWJ=$(last_job_path) &&
n=$(flux kvs get ${LWJ}.nnodes) &&
n=$(flux kvs get --json ${LWJ}.nnodes) &&
test "$n" = "${SIZE}"
'

Expand Down Expand Up @@ -374,7 +374,7 @@ test_expect_success NO_SCHED 'flux-submit: returns ENOSYS when sched not loaded'
check_complete_link() {
for i in `seq 0 5`; do
lastepoch=$(flux kvs dir lwj-complete | awk -F. '{print $2}' | sort -n | tail -1)
flux kvs get lwj-complete.${lastepoch}.${1}.state && return 0
flux kvs get --json lwj-complete.${lastepoch}.${1}.state && return 0
sleep 0.2
done
return 1
Expand Down
Loading

0 comments on commit 2892d92

Please sign in to comment.