You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by achristianson <gi...@git.apache.org> on 2018/05/01 13:46:26 UTC
[GitHub] nifi-minifi-cpp pull request #315: MINIFICPP-472 Added date formatting EL fu...
GitHub user achristianson opened a pull request:
https://github.com/apache/nifi-minifi-cpp/pull/315
MINIFICPP-472 Added date formatting EL functions
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [x] Does your PR title start with MINIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
- [x] Is your initial contribution a single, squashed commit?
### For code changes:
- [x] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [x] If applicable, have you updated the LICENSE file?
- [x] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [x] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/achristianson/nifi-minifi-cpp MINIFICPP-472
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi-minifi-cpp/pull/315.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #315
----
commit 21d937ee61234abcce96c81e770554d572732359
Author: Andrew I. Christianson <an...@...>
Date: 2018-04-30T19:41:17Z
MINIFICPP-472 Added date formatting EL functions
----
---
[GitHub] nifi-minifi-cpp issue #315: MINIFICPP-472 Added date formatting EL functions
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/315
Looks like there are still build issues:
https://api.travis-ci.org/v3/job/379278498/log.txt
https://travis-ci.org/apiri/nifi-minifi-cpp/builds/379278497
---
[GitHub] nifi-minifi-cpp pull request #315: MINIFICPP-472 Added date formatting EL fu...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/315#discussion_r189322613
--- Diff: extensions/expression-language/impl/expression/Expression.h ---
@@ -25,6 +25,11 @@
#undef EXPRESSION_LANGUAGE_USE_REGEX
#endif
+// Disable date in EL for incompatible compilers
--- End diff --
Need to define EXPRESSION_LANGUAGE_USE_DATE
---
[GitHub] nifi-minifi-cpp issue #315: MINIFICPP-472 Added date formatting EL functions
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/315
@achristianson Started reviewing. Build failed on my local envs and it looks like Travis is reproducing the same. Also seems to be present for #325. Could you fix those up and rebase please?
---
[GitHub] nifi-minifi-cpp issue #315: MINIFICPP-472 Added date formatting EL functions
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/315
Looks like we have build issues with that change. I attempted building in an Ubuntu 17.10 container:
```
ExpressionLanguageTests is a Catch v1.6.1 host application.
Run with -? for options
-------------------------------------------------------------------------------
Parse Date
-------------------------------------------------------------------------------
/root/nifi-minifi-cpp/libminifi/test/expression-language-tests/ExpressionLanguageTests.cpp:1216
...............................................................................
/root/nifi-minifi-cpp/libminifi/test/expression-language-tests/ExpressionLanguageTests.cpp:1221: FAILED:
REQUIRE( "1398841200000" == expr({flow_file_a}).asString() )
due to unexpected exception with message:
discover_tz_dir failed to find zoneinfo
-------------------------------------------------------------------------------
Format Date
-------------------------------------------------------------------------------
/root/nifi-minifi-cpp/libminifi/test/expression-language-tests/ExpressionLanguageTests.cpp:1224
...............................................................................
/root/nifi-minifi-cpp/libminifi/test/expression-language-tests/ExpressionLanguageTests.cpp:1229: FAILED:
REQUIRE( "03-14-2014" == expr({flow_file_a}).asString() )
due to unexpected exception with message:
discover_tz_dir failed to find zoneinfo
-------------------------------------------------------------------------------
Reformat Date
-------------------------------------------------------------------------------
/root/nifi-minifi-cpp/libminifi/test/expression-language-tests/ExpressionLanguageTests.cpp:1232
...............................................................................
/root/nifi-minifi-cpp/libminifi/test/expression-language-tests/ExpressionLanguageTests.cpp:1237: FAILED:
REQUIRE( "03-13-2014" == expr({flow_file_a}).asString() )
due to unexpected exception with message:
discover_tz_dir failed to find zoneinfo
-------------------------------------------------------------------------------
Now Date
-------------------------------------------------------------------------------
/root/nifi-minifi-cpp/libminifi/test/expression-language-tests/ExpressionLanguageTests.cpp:1240
...............................................................................
/root/nifi-minifi-cpp/libminifi/test/expression-language-tests/ExpressionLanguageTests.cpp:1249: FAILED:
REQUIRE( (lt.tm_year + 1900) == expr({flow_file_a}).asUnsignedLong() )
due to unexpected exception with message:
discover_tz_dir failed to find zoneinfo
```
---
[GitHub] nifi-minifi-cpp issue #315: MINIFICPP-472 Added date formatting EL functions
Posted by achristianson <gi...@git.apache.org>.
Github user achristianson commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/315
@apiri Confirmed compiler version issue, from Howard Hinnant, author of date:
"I'm not positive to tell you the truth, but based on this bug report: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60396 my best guess is gcc-5 and forward, at least for date::parse. The parse function is implemented in terms of the C++11 std::time_get::get() function(s) proposed by this 2007 paper: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2321.html. There is no workaround, except to manually implement your own parsing. Sorry."
https://gitter.im/HowardHinnant/date/archives/2016/08/31
Will add a version restriction.
---
[GitHub] nifi-minifi-cpp issue #315: MINIFICPP-472 Added date formatting EL functions
Posted by achristianson <gi...@git.apache.org>.
Github user achristianson commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/315
Ah, seems to be in the 3rd party date lib. We might need to disable these date fns when an old compiler is in use (similar to how we disable regex).
The lib should support C++11 though, so will look into it a bit more before making the call.
---
[GitHub] nifi-minifi-cpp issue #315: MINIFICPP-472 Added date formatting EL functions
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/315
@achristianson will scope out in a bit
---
[GitHub] nifi-minifi-cpp issue #315: MINIFICPP-472 Added date formatting EL functions
Posted by achristianson <gi...@git.apache.org>.
Github user achristianson commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/315
@apiri I think that might be more of a container issue vs. a build issue. The date lib interacts with the system zoneinfo db, so I think trouble with that is why it is raising a runtime exception during the test.
---
[GitHub] nifi-minifi-cpp issue #315: MINIFICPP-472 Added date formatting EL functions
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/315
@achristianson ah, fair point. otherwise looks good to go. thanks for the fixes!
---
[GitHub] nifi-minifi-cpp issue #315: MINIFICPP-472 Added date formatting EL functions
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/315
@achristianson Looks like we need to define the date macro. We should probably also clear out the test elements from the thirdparty source.
---
[GitHub] nifi-minifi-cpp pull request #315: MINIFICPP-472 Added date formatting EL fu...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi-minifi-cpp/pull/315
---
[GitHub] nifi-minifi-cpp issue #315: MINIFICPP-472 Added date formatting EL functions
Posted by achristianson <gi...@git.apache.org>.
Github user achristianson commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/315
@apiri looks like travis issue is fixed. Did you see any other issues?
---
[GitHub] nifi-minifi-cpp issue #315: MINIFICPP-472 Added date formatting EL functions
Posted by achristianson <gi...@git.apache.org>.
Github user achristianson commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/315
Taking a look.
---
[GitHub] nifi-minifi-cpp pull request #315: MINIFICPP-472 Added date formatting EL fu...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/315#discussion_r189289574
--- Diff: thirdparty/date/test/clock_cast_test/custom_clock.pass.cpp ---
@@ -0,0 +1,187 @@
+// The MIT License (MIT)
--- End diff --
to minimize what we are carrying with our codebase, could we remove the test subdirectory from our thirdparty?
---
[GitHub] nifi-minifi-cpp issue #315: MINIFICPP-472 Added date formatting EL functions
Posted by achristianson <gi...@git.apache.org>.
Github user achristianson commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/315
@apiri done.
---