[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user jfrazee commented on the issue: https://github.com/apache/nifi/pull/1312 @kedarchitale +1 will merge in the morning. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user kedarchitale commented on the issue: https://github.com/apache/nifi/pull/1312 @jfrazee Vital Signs section has been added to the mapping. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user kedarchitale commented on the issue: https://github.com/apache/nifi/pull/1312 @jfrazee thanks for the second review. the comments have been addressed, let me know if anything has been missed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user jfrazee commented on the issue: https://github.com/apache/nifi/pull/1312 @kedarchitale We're getting pretty close. In addition to the per-line comments I noticed that you have DOS-style CRLF endings. Can you changes these to newlines only using dos2unix or something similar? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user kedarchitale commented on the issue: https://github.com/apache/nifi/pull/1312 @jfrazee thanks for the detailed feedback. all comments have been addressed, do let me know if any further changes are required --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user jfrazee commented on the issue: https://github.com/apache/nifi/pull/1312 @kedarchitale I left some comments on some things that we need to change. Most are small but important modifications, but there's one thing that after some thought I think the processor is doing that it shouldn't: it's not the right place for transforming the output to JSON, especially since we have AttributesToJSON which will make it easy to do the same thing. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user jfrazee commented on the issue: https://github.com/apache/nifi/pull/1312 @kedarchitale Fantastic. Will start digging into the changes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user kedarchitale commented on the issue: https://github.com/apache/nifi/pull/1312 @jfrazee the xml file for testing is no longer required as using mdht api to generate test data is a better approach and even followed in the mdht library. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user jfrazee commented on the issue: https://github.com/apache/nifi/pull/1312 @kedarchitale Thanks for the explanation. I just wanted to make sure we discussed the packaging. The XML file is potentially an issue though. Even if it's an open license, without knowing what the contributor intended the particular license to be, we don't know if it's actually compatible with ASLv2. Since this will end up in the source release we have to be 100% sure about this. I want to get this wrapped up this week so I'm going to try to email @jmandel and @gjewell and see if they can clarify. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user kedarchitale commented on the issue: https://github.com/apache/nifi/pull/1312 @jfrazee The consideration of separate bundle was based on SOLID principle. The HL7 bundle address HL7 V2 which is ascii/text protocol with XML support where as CDA is completely XML based. HL7 V3 is based on RIM model and CDA is one specific domain of V3 for clinical documents. The adoption of HL7 V3 varies as compared to legacy HL7 V2. The underlying parsers are different (HAPI and MDHT). CDA bundle is based on mapping, which will grow as more sections/entries are mapped by contributors. For now, keeping them as separate bundles makes sense, as they can be used/maintained separately without impacting each other. Regarding the XML file, this is used for unit testing only and not shipped with the bundle runtime deployment. This is a Cerner Sample that was shared over github by @jmandel and not clear about the exact origins of this file. The markdown for that repository states that sample documents should be available under an open license, and ASLv2 being would be such an open license. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user jfrazee commented on the issue: https://github.com/apache/nifi/pull/1312 @kedarchitale I'm going to try picking up the review on this so we can get this done. Thanks for already making all the changes Joe suggested! I have two questions now. First, do you think there's any sense in moving this over into the existing HL7 bundle? I don't have strong opinions but I think keeping the HL7 related stuff together would be good. Would love to know your perspective. Second, I can't verify the source + licensing of the Transition_of_Care_Referral_Summary.xml example file that's used. Based on your docs I know you got it from chb/sample_ccdas (thanks for making that clear too) but it's not obvious that we can say it's ASLv2. Tagging @jmandel who I think made the contrib to that repo in case he can help clear it up. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user kedarchitale commented on the issue: https://github.com/apache/nifi/pull/1312 What is the next step on this? Do I have to do anything or this would be reviewed by members of dev community and then merged with the master? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user kedarchitale commented on the issue: https://github.com/apache/nifi/pull/1312 @joewitt, I have moved the contents of README.md to additionalDetails.html --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user kedarchitale commented on the issue: https://github.com/apache/nifi/pull/1312 @joewitt, Thanks for the appreciation and early feedback. Will look into adding documentation to 'additionalDetails'. The properties file contains mappings for CCDA, which would not change once all sections are mapped, however wanted to externalize these from java code, so kept it in the bundle as resources. These properties would not be changed by NiFi user (at least don't foresee a need) and ideally should be changed and tested by contributors mapping any additional sections of CCDA. Considering this, options of having it as a single processor property or externalizing it from the bundle were not suitable. Will post on the dev list seeking feedback from the community. Thanks again for the feedback! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user joewitt commented on the issue: https://github.com/apache/nifi/pull/1312 @kedarchitale I've done quick scan through the contrib so not a thorough evaluation. But I want to immediately say thank you for what is quite obviously a very thoughtful and detailed contribution. You appear to have been very thoughtful about license and notice and following convention around which is extremely appreciated. Nice! I also noticed you provided excellent documentation for the processor. Instead of that being expressed in the readme file could you take a look at some of the example processors that take advantage of the 'additionalDetails' which means this wonderful information becomes part of the automated documentation and available to the user through the application? I noticed there was a properties file in the src/main/resources. I haven't looked into how that ties in but if this is something you'll want the user to be able to edit perhaps we can consider some alternative approaches. One challenge with properties files is that if it is meant to be user editable then it can become cluster unfriendly. So perhaps those things could be expressed as processor properties instead or even as a single processor property. Not sure and perhaps you've already thought through that. Just wanted to mention it. Anyway - great stuff and thanks for contributing! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #1312: NIFI-3147 CCDA Processor
Github user kedarchitale commented on the issue: https://github.com/apache/nifi/pull/1312 CI build passed but AppVeyor build is failing due to unrelated issue Running org.apache.nifi.processors.standard.TestListFile Tests run: 12, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 4.367 sec <<< FAILURE! - in org.apache.nifi.processors.standard.TestListFile testAttributesSet(org.apache.nifi.processors.standard.TestListFile) Time elapsed: 0.203 sec <<< FAILURE! java.lang.AssertionError: null at org.junit.Assert.fail(Assert.java:86) at org.junit.Assert.assertTrue(Assert.java:41) at org.junit.Assert.assertTrue(Assert.java:52) at org.apache.nifi.processors.standard.TestListFile.testAttributesSet(TestListFile.java:675) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
