[GitHub] [nifi] exceptionfactory commented on pull request #7194: NIFI-11167 - Add Excel Record Reader

2023-06-16 Thread via GitHub
exceptionfactory commented on PR #7194: URL: https://github.com/apache/nifi/pull/7194#issuecomment-1595134094 Thanks for moving the `VolatileSchemaCache` back to the record services module. The latest code changes look good. In the course of review, I noticed that `nifi-poi-services`

[GitHub] [nifi] exceptionfactory commented on pull request #7194: NIFI-11167 - Add Excel Record Reader

2023-06-09 Thread via GitHub
exceptionfactory commented on PR #7194: URL: https://github.com/apache/nifi/pull/7194#issuecomment-1584952966 > @exceptionfactory I found that I had to cut and paste files > > 1. org/apache/nifi/schema/inference/CachedSchemaAccessStrategy.java > > 2. org/apache/nifi/sche

[GitHub] [nifi] exceptionfactory commented on pull request #7194: NIFI-11167 - Add Excel Record Reader

2023-06-09 Thread via GitHub
exceptionfactory commented on PR #7194: URL: https://github.com/apache/nifi/pull/7194#issuecomment-1584871036 > @exceptionfactory In order to do this I do need many of the dependencies that are already in `nifi-schema-registry-service-api` as well as classes that are in that module e.g. (`

[GitHub] [nifi] exceptionfactory commented on pull request #7194: NIFI-11167 - Add Excel Record Reader

2023-05-15 Thread via GitHub
exceptionfactory commented on PR #7194: URL: https://github.com/apache/nifi/pull/7194#issuecomment-1548388099 Thanks @dan-s1, this is on my list to review, I plan to take a closer look soon. -- This is an automated message from the Apache Git Service. To respond to the message, please log

[GitHub] [nifi] exceptionfactory commented on pull request #7194: NIFI-11167 - Add Excel Record Reader

2023-04-28 Thread via GitHub
exceptionfactory commented on PR #7194: URL: https://github.com/apache/nifi/pull/7194#issuecomment-1528100343 Thanks @dan-s1! For subsequent changes on this pull request, please add new commits so it we can track the changes necessary. I started the automated build to evaluate the changes.

[GitHub] [nifi] exceptionfactory commented on pull request #7194: NIFI-11167 - Add Excel Record Reader

2023-04-27 Thread via GitHub
exceptionfactory commented on PR #7194: URL: https://github.com/apache/nifi/pull/7194#issuecomment-1526558290 Thanks for improving the tests @dan-s1. The Windows build failure needs to be corrected, can you narrow down why it is failing? -- This is an automated message from the Apache Git

[GitHub] [nifi] exceptionfactory commented on pull request #7194: NIFI-11167 - Add Excel Record Reader

2023-04-27 Thread via GitHub
exceptionfactory commented on PR #7194: URL: https://github.com/apache/nifi/pull/7194#issuecomment-1526060605 > @exceptionfactory Are the locales where these CI builds English speaking? @dan-s1, no the automated builds use different locales (`EN`, `HI`, `JP`, and `FR`) to surface pote

[GitHub] [nifi] exceptionfactory commented on pull request #7194: NIFI-11167 - Add Excel Record Reader

2023-04-25 Thread via GitHub
exceptionfactory commented on PR #7194: URL: https://github.com/apache/nifi/pull/7194#issuecomment-1522435575 @MikeThomsen That's a good question about strict versus lenient parsing. I have not reviewed the pull request in detail yet either, but I plan to review it soon. One primary goal is

[GitHub] [nifi] exceptionfactory commented on pull request #7194: NIFI-11167 - Add Excel Record Reader

2023-04-25 Thread via GitHub
exceptionfactory commented on PR #7194: URL: https://github.com/apache/nifi/pull/7194#issuecomment-1522433481 > @MikeThomsen @exceptionfactory The test failures I see are > > > Error: Failures: > > Error:TestExcelSchemaInference.testInferenceIncludesAllRecords:6

[GitHub] [nifi] exceptionfactory commented on pull request #7194: NIFI-11167 - Add Excel Record Reader

2023-04-25 Thread via GitHub
exceptionfactory commented on PR #7194: URL: https://github.com/apache/nifi/pull/7194#issuecomment-1522036148 > @MikeThomsen @exceptionfactory The test failures I see are > > > Error: Failures: > > Error:TestExcelSchemaInference.testInferenceIncludesAllRecords:62 expected: bu