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

cmd/flux-kvs: Remove legacy --json options and json output #2807

Merged
merged 5 commits into from Mar 9, 2020

Conversation

chu11
Copy link
Member

@chu11 chu11 commented Mar 7, 2020

Per #2796, remove legacy --json options in flux kvs and remove json output from flux kvs dir.

The meat of this PR is the changing on the tests, which is in 1 huge commit. Outside of just removing the --json calls, there had to be tweaks to the tests and the removal of a few that were --json specific.

@chu11
Copy link
Member Author

chu11 commented Mar 7, 2020

Doh, missed that flux-sched has a flux kvs -j call in flux-tree (only searched --json). Will have to push a PR there too.

@dongahn
Copy link
Member

dongahn commented Mar 7, 2020

@SteVwonder tested this and found the following works:

< flux --parent kvs put -j "tree-perf=${blurb}" || warn "KVS error"
> flux --parent kvs put --raw "tree-perf=-" <<< "${blurb}" || warn "KVS error"

@chu11
Copy link
Member Author

chu11 commented Mar 7, 2020

Hmmm, asan builder hung (which I think happens once in awhile), but had one builder just fail with

FAIL: t2007-caliper.t 1 - --caliper-profile works

FAIL: t2007-caliper.t 2 - caliper output file exists

           t2007-caliper.t:  FAIL: N=3   PASS=1   FAIL=2 SKIP=0 XPASS=0 XFAIL=0

ERROR: t2007-caliper.t - exited with status 1

Not really sure about what could have caused this.

also, found typo in a commit message and I forgot 1 --json call in the tests. So rebased & re-pushed.

@chu11
Copy link
Member Author

chu11 commented Mar 7, 2020

Hmmm asan builder got


        t2200-job-ingest.t:  SKIP: N=0   PASS=0   FAIL=0 SKIP=0 XPASS=0 XFAIL=0

ERROR: t2200-job-ingest.t - missing test plan

ERROR: t2200-job-ingest.t - exited with status 1

caused by

ERROR: t2200-job-ingest
=======================

Alarm clock
error: failed to find lua posix module in path
ERROR: t2200-job-ingest.t - missing test plan
ERROR: t2200-job-ingest.t - exited with status 1

Which likely came from

#  Test requirements for testsuite
if ! run_timeout 1.0 lua -e 'require "posix"'; then
    error "failed to find lua posix module in path"
fi

I'm guessing 1.0 seconds timed out ...

Added a patch to increase that timeout, re-pushed PR.

Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup! This looks good to me.

Remove all use of the flux kvs get & put --json option in tests.
Adjust tests appropriately.

In some cases, tests are removed because they were --json specific.

Most adjustments of tests have to do with minor formatting.  For example,
take the following put & get.

flux kvs put --json val="[1,3,5]"
flux kvs get --json val

This test might have previously checked for the output of "[1, 3, 5]"
because internal json libraries would have added spaces when outputting
the json array.  Without --json, the raw "[1,3,5]" input is preserved.
Remove --json option for get & put.

Update manpage appropriately.

Fixes flux-framework#2796
By default, the kvs dir command will output any values that
are in json format in a special json formatting.  Do not do this,
just output normally.

Adjust tests appropriately.
Increase timeout when looking for lua posix module.
@mergify mergify bot merged commit 175b5ed into flux-framework:master Mar 9, 2020
@chu11 chu11 mentioned this pull request Mar 13, 2020
@chu11 chu11 deleted the issue2796 branch June 5, 2021 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants