[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user phrocker commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 @calebj I submitted a PR, though it has the full history. Did you ever create a benchmark. While @achristianson linked the benchmark's on github, I've seen slowdowns of 1-2% when parsing our payloads since they are significantly smaller. ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user phrocker commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 @calebj I've rebased this PR so that it's up to date with master and have addressed some issues. I'll try to submit a PR against PR shortly after I finish some final testing. ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user apiri commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 Hey @calebj @phrocker, Looks like we have a bit of effort to wrap this up remaining so would like to slide this to 0.5.0. Would like for 0.4.0 to be available so we can provide the appropriate tooling to interact with the RPG changes in NiFi 1.5.0. ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user calebj commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 @phrocker Honestly, I just reimplemented the JSON handling in RapidJSON as well as I could, and fixed the things that broke the build and tests. Help would be greatly appreciated. ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user phrocker commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 @calebj HTTP Site to Site and C2 don't seem to work. Do you need help in testing these? ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user achristianson commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 Suggest if we are going to merge it, a good time would be the very start of 0.5.0 so we have plenty of time to test and fix any issues that crop up by the time the next release comes around. ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user phrocker commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 I'm currently not a +1 on this PR. ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user achristianson commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 We should probably make a decision on this soon because this type of change can get out of date fairly easily and JSON touches a lot of things in the codebase. ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user achristianson commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 @calebj we're 100% golden on the licence. We have: - RapidJSON itself under MIT (Apache Category A) - msinttypes r29 under BSD (Apache Category A) - JSON_checker under JSON (Recently banned/put into to Apache Category X) Upon closer inspection of the JSON_checker, the RapidJSON license says this: "To avoid the problematic JSON license in your own projects, it's sufficient to exclude the bin/jsonchecker/ directory, as it's the only code under the JSON license." So, strip out the bin/jsonchecker/ dir and we're fully in Category A territory. ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user calebj commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 I know that it's MIT licensed, but their license.txt comes with others tacked on for subcomponents. If you think they're all compatible, I'll copy all of them in. ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user achristianson commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 @phrocker There are some benchmarks over here: https://github.com/miloyip/nativejson-benchmark ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user calebj commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 @achristianson something in the master branch is failing. Looks like something in the commit before the latest one, 9b9c3354330d525cfdd50f656ae42fc3da80764c, broke it. Regarding the LICENSE, there are a handful of them. IANAL, so can someone look over [rapidjson's license.txt](https://github.com/Tencent/rapidjson/blob/master/license.txt) and see what might have to be left out, if anything? ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user achristianson commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 s2s integration tests for this are passing (after a few docker int test fixes --see other PRs). ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user achristianson commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 Looks like a test failure in the travis xcode: ``` The following tests FAILED: 38 - CompressContentTests (Failed) ``` ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user phrocker commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 @calebj Thanks for the contribution, but before this can be merged can you provide us some tractable benchmarks? Are we making a library change just to make a library change based on evidence? https://github.com/google/benchmark is a great tool that will help. ---
[GitHub] nifi-minifi-cpp issue #191: MINIFICPP-114 Consolidate JSON API use to RapidJ...
Github user achristianson commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/191 @calebj whoa, nice!!! This is something I have been thinking about for a while, but was really back burner. This is great, will check it out and let you know if I find anything. ---
