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

Feature/jq #2572

Merged
merged 2 commits into from
Apr 20, 2016
Merged

Feature/jq #2572

merged 2 commits into from
Apr 20, 2016

Conversation

kacf
Copy link
Contributor

@kacf kacf commented Apr 12, 2016

Combination of #2319 and #2571, just for testing. Please review the mentioned PRs individually.

@kacf kacf mentioned this pull request Apr 12, 2016
@kacf
Copy link
Contributor Author

kacf commented Apr 12, 2016

trigger build

@kacf
Copy link
Contributor Author

kacf commented Apr 13, 2016

This now instead fails in a lot of platforms with:

 error: Function mapdata failed to get output from "json_pipe" execution
   error: Function mapdata failed to get output from "json_pipe" execution
   error: Function mapdata failed to get output from "json_pipe" execution
R: DEBUG file_make: creating /home/jenkins/workspace/testing-enterprise-pr_core/label/PACKAGES_i386_linux_debian_4/core/tests/acceptance/workdir/__01_vars_02_functions_mapdata_json_pipe_cf/tmp/TESTDIR.cfengine/template with contents "{{%state}}"
R: DEBUG file_make: creating /home/jenkins/workspace/testing-enterprise-pr_core/label/PACKAGES_i386_linux_debian_4/core/tests/acceptance/workdir/__01_vars_02_functions_mapdata_json_pipe_cf/tmp/TESTDIR.cfengine/actual with contents ""
R: FILES DIFFER BUT SHOULD BE THE SAME
R: CONTENTS OF /home/jenkins/workspace/testing-enterprise-pr_core/label/PACKAGES_i386_linux_debian_4/core/tests/acceptance/workdir/__01_vars_02_functions_mapdata_json_pipe_cf/tmp/TESTDIR.cfengine/actual:
{
  "items": {
    "titles": [
      "JQ Primer",
      "More JQ"
    ],
    "user": "stedolan"
  },
  "items_jqinput": [
    {
      "title": "JQ Primer",
      "user": "stedolan"
    },
    {
      "title": "More JQ",
      "user": "stedolan"
    }
  ],
  "items_passthrough": [
    {
      "titles": [
        "JQ Primer",
        "More JQ"
      ],
      "user": "stedolan"
    }
  ]
}

R: CONTENTS OF /home/jenkins/workspace/testing-enterprise-pr_core/label/PACKAGES_i386_linux_debian_4/core/tests/acceptance/./01_vars/02_functions/mapdata_json_pipe.cf.expected.json:
{
  "items": {
    "titles": [
      "JQ Primer",
      "More JQ"
    ],
    "user": "stedolan"
  },
  "items_input": [
    {
      "a": 100,
      "b": true
    },
    {
      "a": 100,
      "b": true
    },
    {
      "a": 100,
      "b": true
    },
    {
    }
  ],
  "items_jqinput": [
    {
      "title": "JQ Primer",
      "user": "stedolan"
    },
    {
      "title": "More JQ",
      "user": "stedolan"
    }
  ],
  "items_passthrough": [
    {
      "titles": [
        "JQ Primer",
        "More JQ"
      ],
      "user": "stedolan"
    }
  ]
}

R: --- /home/jenkins/workspace/testing-enterprise-pr_core/label/PACKAGES_i386_linux_debian_4/core/tests/acceptance/./01_vars/02_functions/mapdata_json_pipe.cf.expected.json    2016-04-12 10:51:37.000000000 +0200
+++ /home/jenkins/workspace/testing-enterprise-pr_core/label/PACKAGES_i386_linux_debian_4/core/tests/acceptance/workdir/__01_vars_02_functions_mapdata_json_pipe_cf/tmp/TESTDIR.cfengine/actual 2016-04-12 11:07:49.000000000 +0200
@@ -6,22 +6,6 @@
     ],
     "user": "stedolan"
   },
-  "items_input": [
-    {
-      "a": 100,
-      "b": true
-    },
-    {
-      "a": 100,
-      "b": true
-    },
-    {
-      "a": 100,
-      "b": true
-    },
-    {
-    }
-  ],
   "items_jqinput": [
     {
       "title": "JQ Primer",
R: /home/jenkins/workspace/testing-enterprise-pr_core/label/PACKAGES_i386_linux_debian_4/core/tests/acceptance/./01_vars/02_functions/mapdata_json_pipe.cf FAIL

I suspect that this is the same underlying problem, but now failing in a more appropriate manner (not crashing): Using cat somefile is inappropriate in the test because cat will not consume the input, which it needs to in order for CFEngine not to complain.

@kacf
Copy link
Contributor Author

kacf commented Apr 14, 2016

trigger build

@kacf
Copy link
Contributor Author

kacf commented Apr 15, 2016

Gah! Now Windows... The compilation fight continues.

@kacf
Copy link
Contributor Author

kacf commented Apr 18, 2016

Ok, I think Windows should be ok now, I moved most of the pipes_unix.c code to pipes.c, since it's not actually platform specific (although cf_popen_full_duplex is). Running this through another round of testing on a private branch: http://jenkins.usdc.cfengine.com/view/Enterprise%20private%20branches/job/testing-kristian-privateBranch/153/

@kacf
Copy link
Contributor Author

kacf commented Apr 18, 2016

I think you can probably get started on the docs. It's only a matter of time before this goes in! :-)

@kacf kacf self-assigned this Apr 18, 2016
@kacf
Copy link
Contributor Author

kacf commented Apr 19, 2016

Alright, I still need to clean up the commit messages, but together with https://github.com/cfengine/enterprise/pull/308 this is green. Just awaiting docs.

@tzz
Copy link
Contributor

tzz commented Apr 19, 2016

@kacf I added an example in df781a3 which I'd like to use in the docs. Can you check if it's OK and bring it into your PR here? Or use #2319, I took all your commits from here and added the example at the end.

@kacf
Copy link
Contributor Author

kacf commented Apr 19, 2016

Great! I think we can't run it in the examples test, there's just too many strings to pull to make it available on every build slave. But if you've verified it locally, we can disable that part and it should be a good example nevertheless.

@tzz
Copy link
Contributor

tzz commented Apr 19, 2016

OK, I changed the example (new commit ID is d51b0b9 ) so the output won't get tested (remake_outputs.pl only looks for the example_output line), but we can still include it in the docs. Let me know if it's OK that way and I'll proceed with the docs.

@tzz
Copy link
Contributor

tzz commented Apr 19, 2016

docs in cfengine/documentation#1450

tzz added 2 commits April 20, 2016 10:36
Squashed from:
- jq integration for mapdata()
- Close hanging file descriptors in ExecJSON_Pipe()
- Use JsonDestroyMaybe() in FnCallMapData()
- Accept multiple lines in ExecJSON_Pipe()
- Add acceptance test for mapdata(json_pipe)

Modified by: Kristian Amlie <kristian.amlie@cfengine.com>
- Fix test by consuming input before outputting file.
- Move Windows functions.
- Fix log messages to no longer refer to package modules.
- Move pipe functions to common file, since they are not UNIX specific.

Changelog: Add `json_pipe` mode to `mapdata()`, which allows piping a
JSON container to an external program for manipulation and receiving
JSON back. The `jq` tool is a good example where this mode can be
useful. A corresponding `$(def.jq)` variable has also been added with
a default path to this tool. See documentation for `mapdata()` for
more information and examples.
@kacf kacf merged commit 9e3a78c into cfengine:master Apr 20, 2016
@kacf kacf deleted the feature/jq branch April 20, 2016 09:01
@kacf
Copy link
Contributor Author

kacf commented Apr 20, 2016

Alright, it's finally in! Thanks for the hard work!

@kacf
Copy link
Contributor Author

kacf commented Apr 20, 2016

Oops, forgot example. Cherry-picked and pushed as well.

@tzz
Copy link
Contributor

tzz commented Apr 20, 2016

woo hoo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants