You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by apiri <gi...@git.apache.org> on 2015/12/01 23:41:07 UTC

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

GitHub user apiri opened a pull request:

    https://github.com/apache/nifi/pull/136

    NIFI-1234 Correcting container functionality for single Avro records.

    @joemeszaros Had a user open an issue concerning the processor's handling of events with only one record.  I agree that the functionality seems a bit odd and was wondering if there was a specific case that would be missed should the containerOption be the only item to dictate format.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/apiri/incubator-nifi NIFI-1234

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi/pull/136.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 #136
    
----
commit 4e07619fd48737b003361070a583330abd2e5d8b
Author: Aldrin Piri <al...@apache.org>
Date:   2015-12-01T22:38:46Z

    NIFI-1234 Correcting container functionality for single Avro records.

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/136#discussion_r46500474
  
    --- Diff: nifi-nar-bundles/nifi-avro-bundle/nifi-avro-processors/src/main/java/org/apache/nifi/processors/avro/ConvertAvroToJSON.java ---
    @@ -116,51 +124,59 @@ public void onTrigger(ProcessContext context, ProcessSession session) throws Pro
             }
     
             final String containerOption = context.getProperty(CONTAINER_OPTIONS).getValue();
    +        final boolean useContainer = containerOption.equals(CONTAINER_ARRAY);
    +        // Wrap a single record (inclusive of no records) only when a container is being used
    +        final boolean wrapSingleRecord = context.getProperty(WRAP_SINGLE_RECORD).asBoolean() && useContainer;
     
             try {
                 flowFile = session.write(flowFile, new StreamCallback() {
                     @Override
                     public void process(final InputStream rawIn, final OutputStream rawOut) throws IOException {
                         try (final InputStream in = new BufferedInputStream(rawIn);
    -
    -                        final OutputStream out = new BufferedOutputStream(rawOut);
    -                        final DataFileStream<GenericRecord> reader = new DataFileStream<>(in, new GenericDatumReader<GenericRecord>())) {
    +                         final OutputStream out = new BufferedOutputStream(rawOut);
    +                         final DataFileStream<GenericRecord> reader = new DataFileStream<>(in, new GenericDatumReader<GenericRecord>())) {
     
                             final GenericData genericData = GenericData.get();
     
    -                        if (reader.hasNext() == false ) {
    -                            out.write(EMPTY_JSON_OBJECT);
    -                            return;
    +                        int recordCount = 0;
    +                        GenericRecord currRecord = null;
    +                        if (reader.hasNext()) {
    +                            currRecord = reader.next();
    +                            recordCount++;
                             }
    -                        int recordCount = 1;
    -                        GenericRecord reuse = reader.next();
    -                        // Only open container if more than one record
    -                        if(reader.hasNext() && containerOption.equals(CONTAINER_ARRAY)){
    +
    +                        // Open container if desired output is an array format and there are are multiple records or
    +                        // if configured to wrap single record
    +                        if (reader.hasNext() && useContainer || wrapSingleRecord) {
                                 out.write('[');
                             }
    -                        out.write(genericData.toString(reuse).getBytes(StandardCharsets.UTF_8));
    +
    +                        // Determine the initial output record, inclusive if we should have an empty set of Avro records
    +                        final byte[] outputBytes = (currRecord == null) ? EMPTY_JSON_OBJECT : genericData.toString(currRecord).getBytes(StandardCharsets.UTF_8);
    +                        out.write(outputBytes);
     
                             while (reader.hasNext()) {
    -                            if (containerOption.equals(CONTAINER_ARRAY)) {
    +                            if (useContainer) {
                                     out.write(',');
                                 } else {
                                     out.write('\n');
                                 }
     
    -                            reuse = reader.next(reuse);
    -                            out.write(genericData.toString(reuse).getBytes(StandardCharsets.UTF_8));
    +                            currRecord = reader.next(currRecord);
    +                            out.write(genericData.toString(currRecord).getBytes(StandardCharsets.UTF_8));
                                 recordCount++;
                             }
     
    -                        // Only close container if more than one record
    -                        if (recordCount > 1 && containerOption.equals(CONTAINER_ARRAY)) {
    +                        // Close container if desired output is an array format and there are multiple records or if
    +                        // configured to wrap a single record
    +                        if (recordCount > 1 && useContainer || wrapSingleRecord) {
    --- End diff --
    
    I think it is correct.
    
    useContainer only applies to the case where there is more than one record.  Those cases where there are zero or one are driven by wrapSingleRecord which incorporates whether or not the container should be used.  Happy to rework this (and the grouping) if I can get some better clarity.   Could certainly alleviate the line where it is defined earlier.
    
    wrapSingleRecord is defined as:
    final boolean wrapSingleRecord = context.getProperty(WRAP_SINGLE_RECORD).asBoolean() && useContainer;



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the pull request:

    https://github.com/apache/nifi/pull/136#issuecomment-161330101
  
    @trkurc 
    I am failing at the Github and JIRA comment game, but as commented on the issue (and I am additionally editing myself for context here):
    
    Sorry, missed some of your earlier comments (these were from last night when you expanded the cases) in flipping between here and Github. Also hate the property bloat, but think it is necessary to avoid the breaking change. Had a hard time grokking the use cases that steered it away from bug status, so am onboard with your viewpoint.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/136#discussion_r46499759
  
    --- Diff: nifi-nar-bundles/nifi-avro-bundle/nifi-avro-processors/src/main/java/org/apache/nifi/processors/avro/ConvertAvroToJSON.java ---
    @@ -116,51 +124,59 @@ public void onTrigger(ProcessContext context, ProcessSession session) throws Pro
             }
     
             final String containerOption = context.getProperty(CONTAINER_OPTIONS).getValue();
    +        final boolean useContainer = containerOption.equals(CONTAINER_ARRAY);
    +        // Wrap a single record (inclusive of no records) only when a container is being used
    +        final boolean wrapSingleRecord = context.getProperty(WRAP_SINGLE_RECORD).asBoolean() && useContainer;
     
             try {
                 flowFile = session.write(flowFile, new StreamCallback() {
                     @Override
                     public void process(final InputStream rawIn, final OutputStream rawOut) throws IOException {
                         try (final InputStream in = new BufferedInputStream(rawIn);
    -
    -                        final OutputStream out = new BufferedOutputStream(rawOut);
    -                        final DataFileStream<GenericRecord> reader = new DataFileStream<>(in, new GenericDatumReader<GenericRecord>())) {
    +                         final OutputStream out = new BufferedOutputStream(rawOut);
    +                         final DataFileStream<GenericRecord> reader = new DataFileStream<>(in, new GenericDatumReader<GenericRecord>())) {
     
                             final GenericData genericData = GenericData.get();
     
    -                        if (reader.hasNext() == false ) {
    -                            out.write(EMPTY_JSON_OBJECT);
    -                            return;
    +                        int recordCount = 0;
    +                        GenericRecord currRecord = null;
    +                        if (reader.hasNext()) {
    +                            currRecord = reader.next();
    +                            recordCount++;
                             }
    -                        int recordCount = 1;
    -                        GenericRecord reuse = reader.next();
    -                        // Only open container if more than one record
    -                        if(reader.hasNext() && containerOption.equals(CONTAINER_ARRAY)){
    +
    +                        // Open container if desired output is an array format and there are are multiple records or
    +                        // if configured to wrap single record
    +                        if (reader.hasNext() && useContainer || wrapSingleRecord) {
                                 out.write('[');
                             }
    -                        out.write(genericData.toString(reuse).getBytes(StandardCharsets.UTF_8));
    +
    +                        // Determine the initial output record, inclusive if we should have an empty set of Avro records
    +                        final byte[] outputBytes = (currRecord == null) ? EMPTY_JSON_OBJECT : genericData.toString(currRecord).getBytes(StandardCharsets.UTF_8);
    +                        out.write(outputBytes);
     
                             while (reader.hasNext()) {
    -                            if (containerOption.equals(CONTAINER_ARRAY)) {
    +                            if (useContainer) {
                                     out.write(',');
                                 } else {
                                     out.write('\n');
                                 }
     
    -                            reuse = reader.next(reuse);
    -                            out.write(genericData.toString(reuse).getBytes(StandardCharsets.UTF_8));
    +                            currRecord = reader.next(currRecord);
    +                            out.write(genericData.toString(currRecord).getBytes(StandardCharsets.UTF_8));
                                 recordCount++;
                             }
     
    -                        // Only close container if more than one record
    -                        if (recordCount > 1 && containerOption.equals(CONTAINER_ARRAY)) {
    +                        // Close container if desired output is an array format and there are multiple records or if
    +                        // configured to wrap a single record
    +                        if (recordCount > 1 && useContainer || wrapSingleRecord) {
    --- End diff --
    
    For next time, comment is good, but parens would make the logic more clear for the next coder to come along:
    ```
    if ((recordCount > 1 && useContainer) || wrapSingleRecord) {
    ```



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/nifi/pull/136


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on the pull request:

    https://github.com/apache/nifi/pull/136#issuecomment-161287833
  
    I am on the side of @apiri. I can't imagine that was the desirable behavior either, since it was simply inconsistent. If one **explicitly** requests an array, than it should return an array, no matter how many elements in it. 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the pull request:

    https://github.com/apache/nifi/pull/136#issuecomment-161289671
  
    It covers the case of records but not record.  I read the documentation to mean that as long as the input flowfile was of Avro formatted record(s), it would perform the associated conversion but admittedly the comments in the code provide much more context.
    
    Regardless, have updated the comments and pushed the branch.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/136#discussion_r46501389
  
    --- Diff: nifi-nar-bundles/nifi-avro-bundle/nifi-avro-processors/src/main/java/org/apache/nifi/processors/avro/ConvertAvroToJSON.java ---
    @@ -116,51 +124,59 @@ public void onTrigger(ProcessContext context, ProcessSession session) throws Pro
             }
     
             final String containerOption = context.getProperty(CONTAINER_OPTIONS).getValue();
    +        final boolean useContainer = containerOption.equals(CONTAINER_ARRAY);
    +        // Wrap a single record (inclusive of no records) only when a container is being used
    +        final boolean wrapSingleRecord = context.getProperty(WRAP_SINGLE_RECORD).asBoolean() && useContainer;
     
             try {
                 flowFile = session.write(flowFile, new StreamCallback() {
                     @Override
                     public void process(final InputStream rawIn, final OutputStream rawOut) throws IOException {
                         try (final InputStream in = new BufferedInputStream(rawIn);
    -
    -                        final OutputStream out = new BufferedOutputStream(rawOut);
    -                        final DataFileStream<GenericRecord> reader = new DataFileStream<>(in, new GenericDatumReader<GenericRecord>())) {
    +                         final OutputStream out = new BufferedOutputStream(rawOut);
    +                         final DataFileStream<GenericRecord> reader = new DataFileStream<>(in, new GenericDatumReader<GenericRecord>())) {
     
                             final GenericData genericData = GenericData.get();
     
    -                        if (reader.hasNext() == false ) {
    -                            out.write(EMPTY_JSON_OBJECT);
    -                            return;
    +                        int recordCount = 0;
    +                        GenericRecord currRecord = null;
    +                        if (reader.hasNext()) {
    +                            currRecord = reader.next();
    +                            recordCount++;
                             }
    -                        int recordCount = 1;
    -                        GenericRecord reuse = reader.next();
    -                        // Only open container if more than one record
    -                        if(reader.hasNext() && containerOption.equals(CONTAINER_ARRAY)){
    +
    +                        // Open container if desired output is an array format and there are are multiple records or
    +                        // if configured to wrap single record
    +                        if (reader.hasNext() && useContainer || wrapSingleRecord) {
                                 out.write('[');
                             }
    -                        out.write(genericData.toString(reuse).getBytes(StandardCharsets.UTF_8));
    +
    +                        // Determine the initial output record, inclusive if we should have an empty set of Avro records
    +                        final byte[] outputBytes = (currRecord == null) ? EMPTY_JSON_OBJECT : genericData.toString(currRecord).getBytes(StandardCharsets.UTF_8);
    +                        out.write(outputBytes);
     
                             while (reader.hasNext()) {
    -                            if (containerOption.equals(CONTAINER_ARRAY)) {
    +                            if (useContainer) {
                                     out.write(',');
                                 } else {
                                     out.write('\n');
                                 }
     
    -                            reuse = reader.next(reuse);
    -                            out.write(genericData.toString(reuse).getBytes(StandardCharsets.UTF_8));
    +                            currRecord = reader.next(currRecord);
    +                            out.write(genericData.toString(currRecord).getBytes(StandardCharsets.UTF_8));
                                 recordCount++;
                             }
     
    -                        // Only close container if more than one record
    -                        if (recordCount > 1 && containerOption.equals(CONTAINER_ARRAY)) {
    +                        // Close container if desired output is an array format and there are multiple records or if
    +                        // configured to wrap a single record
    +                        if (recordCount > 1 && useContainer || wrapSingleRecord) {
    --- End diff --
    
    @apiri how logic flows through code is a personal preference. I was confused for a spell, but there were unit tests checking the behavior. 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/136#issuecomment-161487382
  
    @olegz - I am also guilty of missing comments between github and jira. the choice of array or none for container wasn't in 0.3.0, so it wasn't an explicit option then. I was basing a lot of my assumptions for "correct" behavior on the original patch submitted for NIFI-855 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/136#discussion_r46500975
  
    --- Diff: nifi-nar-bundles/nifi-avro-bundle/nifi-avro-processors/src/main/java/org/apache/nifi/processors/avro/ConvertAvroToJSON.java ---
    @@ -116,51 +124,59 @@ public void onTrigger(ProcessContext context, ProcessSession session) throws Pro
             }
     
             final String containerOption = context.getProperty(CONTAINER_OPTIONS).getValue();
    +        final boolean useContainer = containerOption.equals(CONTAINER_ARRAY);
    +        // Wrap a single record (inclusive of no records) only when a container is being used
    +        final boolean wrapSingleRecord = context.getProperty(WRAP_SINGLE_RECORD).asBoolean() && useContainer;
     
             try {
                 flowFile = session.write(flowFile, new StreamCallback() {
                     @Override
                     public void process(final InputStream rawIn, final OutputStream rawOut) throws IOException {
                         try (final InputStream in = new BufferedInputStream(rawIn);
    -
    -                        final OutputStream out = new BufferedOutputStream(rawOut);
    -                        final DataFileStream<GenericRecord> reader = new DataFileStream<>(in, new GenericDatumReader<GenericRecord>())) {
    +                         final OutputStream out = new BufferedOutputStream(rawOut);
    +                         final DataFileStream<GenericRecord> reader = new DataFileStream<>(in, new GenericDatumReader<GenericRecord>())) {
     
                             final GenericData genericData = GenericData.get();
     
    -                        if (reader.hasNext() == false ) {
    -                            out.write(EMPTY_JSON_OBJECT);
    -                            return;
    +                        int recordCount = 0;
    +                        GenericRecord currRecord = null;
    +                        if (reader.hasNext()) {
    +                            currRecord = reader.next();
    +                            recordCount++;
                             }
    -                        int recordCount = 1;
    -                        GenericRecord reuse = reader.next();
    -                        // Only open container if more than one record
    -                        if(reader.hasNext() && containerOption.equals(CONTAINER_ARRAY)){
    +
    +                        // Open container if desired output is an array format and there are are multiple records or
    +                        // if configured to wrap single record
    +                        if (reader.hasNext() && useContainer || wrapSingleRecord) {
                                 out.write('[');
                             }
    -                        out.write(genericData.toString(reuse).getBytes(StandardCharsets.UTF_8));
    +
    +                        // Determine the initial output record, inclusive if we should have an empty set of Avro records
    +                        final byte[] outputBytes = (currRecord == null) ? EMPTY_JSON_OBJECT : genericData.toString(currRecord).getBytes(StandardCharsets.UTF_8);
    +                        out.write(outputBytes);
     
                             while (reader.hasNext()) {
    -                            if (containerOption.equals(CONTAINER_ARRAY)) {
    +                            if (useContainer) {
                                     out.write(',');
                                 } else {
                                     out.write('\n');
                                 }
     
    -                            reuse = reader.next(reuse);
    -                            out.write(genericData.toString(reuse).getBytes(StandardCharsets.UTF_8));
    +                            currRecord = reader.next(currRecord);
    +                            out.write(genericData.toString(currRecord).getBytes(StandardCharsets.UTF_8));
                                 recordCount++;
                             }
     
    -                        // Only close container if more than one record
    -                        if (recordCount > 1 && containerOption.equals(CONTAINER_ARRAY)) {
    +                        // Close container if desired output is an array format and there are multiple records or if
    +                        // configured to wrap a single record
    +                        if (recordCount > 1 && useContainer || wrapSingleRecord) {
    --- End diff --
    
    I see now from line 127 that this is what is happening


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/136#discussion_r46500106
  
    --- Diff: nifi-nar-bundles/nifi-avro-bundle/nifi-avro-processors/src/main/java/org/apache/nifi/processors/avro/ConvertAvroToJSON.java ---
    @@ -116,51 +124,59 @@ public void onTrigger(ProcessContext context, ProcessSession session) throws Pro
             }
     
             final String containerOption = context.getProperty(CONTAINER_OPTIONS).getValue();
    +        final boolean useContainer = containerOption.equals(CONTAINER_ARRAY);
    +        // Wrap a single record (inclusive of no records) only when a container is being used
    +        final boolean wrapSingleRecord = context.getProperty(WRAP_SINGLE_RECORD).asBoolean() && useContainer;
     
             try {
                 flowFile = session.write(flowFile, new StreamCallback() {
                     @Override
                     public void process(final InputStream rawIn, final OutputStream rawOut) throws IOException {
                         try (final InputStream in = new BufferedInputStream(rawIn);
    -
    -                        final OutputStream out = new BufferedOutputStream(rawOut);
    -                        final DataFileStream<GenericRecord> reader = new DataFileStream<>(in, new GenericDatumReader<GenericRecord>())) {
    +                         final OutputStream out = new BufferedOutputStream(rawOut);
    +                         final DataFileStream<GenericRecord> reader = new DataFileStream<>(in, new GenericDatumReader<GenericRecord>())) {
     
                             final GenericData genericData = GenericData.get();
     
    -                        if (reader.hasNext() == false ) {
    -                            out.write(EMPTY_JSON_OBJECT);
    -                            return;
    +                        int recordCount = 0;
    +                        GenericRecord currRecord = null;
    +                        if (reader.hasNext()) {
    +                            currRecord = reader.next();
    +                            recordCount++;
                             }
    -                        int recordCount = 1;
    -                        GenericRecord reuse = reader.next();
    -                        // Only open container if more than one record
    -                        if(reader.hasNext() && containerOption.equals(CONTAINER_ARRAY)){
    +
    +                        // Open container if desired output is an array format and there are are multiple records or
    +                        // if configured to wrap single record
    +                        if (reader.hasNext() && useContainer || wrapSingleRecord) {
                                 out.write('[');
                             }
    -                        out.write(genericData.toString(reuse).getBytes(StandardCharsets.UTF_8));
    +
    +                        // Determine the initial output record, inclusive if we should have an empty set of Avro records
    +                        final byte[] outputBytes = (currRecord == null) ? EMPTY_JSON_OBJECT : genericData.toString(currRecord).getBytes(StandardCharsets.UTF_8);
    +                        out.write(outputBytes);
     
                             while (reader.hasNext()) {
    -                            if (containerOption.equals(CONTAINER_ARRAY)) {
    +                            if (useContainer) {
                                     out.write(',');
                                 } else {
                                     out.write('\n');
                                 }
     
    -                            reuse = reader.next(reuse);
    -                            out.write(genericData.toString(reuse).getBytes(StandardCharsets.UTF_8));
    +                            currRecord = reader.next(currRecord);
    +                            out.write(genericData.toString(currRecord).getBytes(StandardCharsets.UTF_8));
                                 recordCount++;
                             }
     
    -                        // Only close container if more than one record
    -                        if (recordCount > 1 && containerOption.equals(CONTAINER_ARRAY)) {
    +                        // Close container if desired output is an array format and there are multiple records or if
    +                        // configured to wrap a single record
    +                        if (recordCount > 1 && useContainer || wrapSingleRecord) {
    --- End diff --
    
    actually, isn't that the wrong logic? shouldn't it be 
    
    if (useContainer && (recordCount > 1 || wrapSingleRecord))


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the pull request:

    https://github.com/apache/nifi/pull/136#issuecomment-161357053
  
    Made the changes to include a property which defaults to the old behavior but enables control over whether a single record is placed in a container


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/136#issuecomment-161179877
  
    both the code and processor description strongly indicated this was intentional.
    
    "If an incoming FlowFile contains a stream of multiple Avro records, the resultant FlowFile will contain a JSON Array containing all of the Avro records."


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/136#issuecomment-161325906
  
    So, some evidence that this was intentional. 
    
    1) it is *harder* to handle the single record case without arrays. @markap14 appears to have contributed this as part of NIFI-855 (even though @joemeszaros added the comments and was the last to touch these lines of code)
    2) the processor description clearly states it has different behavior with multiple records
    3) It makes sense to me that if you were administering a flow that only ever had a single avro record per flow file, you would NOT want it inside an array
    
    I believe 3 is a valid use case, and this change may break flows. I highly recommend adding a flag to preserve this behavior. I explained more in jira (https://issues.apache.org/jira/browse/NIFI-1234)



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1234 Correcting container functionality fo...

Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the pull request:

    https://github.com/apache/nifi/pull/136#issuecomment-161178953
  
    @trkurc good catches on the comments. Overall would you feel like this is a bug? That's the impression I get and trying to think if this is adjusted is it breaking? I imagine it could be but have a hard time envisioning that behavior being desirable. Thoughts?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---