You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "joewitt (via GitHub)" <gi...@apache.org> on 2024/05/09 00:39:23 UTC
[PR] NIFI-13186 corrected required runtime dep lucene-backward-codecs and … [nifi]
joewitt opened a new pull request, #8782:
URL: https://github.com/apache/nifi/pull/8782
…updated tests
<!-- Licensed to the Apache Software Foundation (ASF) under one or more -->
<!-- contributor license agreements. See the NOTICE file distributed with -->
<!-- this work for additional information regarding copyright ownership. -->
<!-- The ASF licenses this file to You under the Apache License, Version 2.0 -->
<!-- (the "License"); you may not use this file except in compliance with -->
<!-- the License. You may obtain a copy of the License at -->
<!-- http://www.apache.org/licenses/LICENSE-2.0 -->
<!-- Unless required by applicable law or agreed to in writing, software -->
<!-- distributed under the License is distributed on an "AS IS" BASIS, -->
<!-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -->
<!-- See the License for the specific language governing permissions and -->
<!-- limitations under the License. -->
# Summary
[NIFI-13186](https://issues.apache.org/jira/browse/NIFI-13186)
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [ ] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
- [ ] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
### Pull Request Formatting
- [ ] Pull Request based on current revision of the `main` branch
- [ ] Pull Request refers to a feature branch with one commit containing changes
# Verification
Please indicate the verification steps performed prior to pull request creation.
### Build
- [ ] Build completed using `mvn clean install -P contrib-check`
- [ ] JDK 21
### UI Contributions
- [ ] NiFi is modernizing its UI. Any contributions that update the [current UI](https://github.com/apache/nifi/tree/main/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui) also need to be implemented in the [new UI](https://github.com/apache/nifi/tree/main/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi).
### Licensing
- [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13186 corrected required runtime dep lucene-backward-codecs and … [nifi]
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #8782:
URL: https://github.com/apache/nifi/pull/8782#discussion_r1595514675
##########
nifi-extension-bundles/nifi-provenance-repository-bundle/pom.xml:
##########
@@ -46,16 +46,6 @@
<artifactId>lucene-core</artifactId>
<version>${lucene.version}</version>
</dependency>
- <dependency>
- <groupId>org.apache.lucene</groupId>
- <artifactId>lucene-analyzers-common</artifactId>
- <version>${lucene.version}</version>
- </dependency>
- <dependency>
- <groupId>org.apache.lucene</groupId>
- <artifactId>lucene-queryparser</artifactId>
- <version>${lucene.version}</version>
- </dependency>
Review Comment:
Thanks for the helpful background and verification, sounds good!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13186 corrected required runtime dep lucene-backward-codecs and … [nifi]
Posted by "joewitt (via GitHub)" <gi...@apache.org>.
joewitt commented on PR #8782:
URL: https://github.com/apache/nifi/pull/8782#issuecomment-2101721029
Validating against m2 built index (lucene 8.11.3) vs this PR. Will share more details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13186 corrected required runtime dep lucene-backward-codecs and … [nifi]
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory closed pull request #8782: NIFI-13186 corrected required runtime dep lucene-backward-codecs and …
URL: https://github.com/apache/nifi/pull/8782
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13186 corrected required runtime dep lucene-backward-codecs and … [nifi]
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #8782:
URL: https://github.com/apache/nifi/pull/8782#discussion_r1595365571
##########
nifi-extension-bundles/nifi-provenance-repository-bundle/nifi-persistent-provenance-repository/src/main/java/org/apache/nifi/provenance/index/lucene/LuceneEventIndex.java:
##########
@@ -182,14 +182,7 @@ private void triggerReindexOfDefunctIndices() {
for (final File defunctIndex : defunctIndices) {
try {
- if (isLucene4IndexPresent(defunctIndex)) {
- logger.info("Encountered Lucene 8 index {} and also the corresponding Lucene 4 index; will only trigger rebuilding of one directory.", defunctIndex);
- rebuildCount.incrementAndGet();
- continue;
- }
Review Comment:
This rebuild step is no longer required?
##########
nifi-extension-bundles/nifi-provenance-repository-bundle/pom.xml:
##########
@@ -46,16 +46,6 @@
<artifactId>lucene-core</artifactId>
<version>${lucene.version}</version>
</dependency>
- <dependency>
- <groupId>org.apache.lucene</groupId>
- <artifactId>lucene-analyzers-common</artifactId>
- <version>${lucene.version}</version>
- </dependency>
- <dependency>
- <groupId>org.apache.lucene</groupId>
- <artifactId>lucene-queryparser</artifactId>
- <version>${lucene.version}</version>
- </dependency>
Review Comment:
Is there a particular reason for removing these two dependencies?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13186 corrected required runtime dep lucene-backward-codecs and … [nifi]
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #8782:
URL: https://github.com/apache/nifi/pull/8782#discussion_r1595515734
##########
nifi-extension-bundles/nifi-provenance-repository-bundle/nifi-persistent-provenance-repository/src/main/java/org/apache/nifi/provenance/index/lucene/LuceneEventIndex.java:
##########
@@ -182,14 +182,7 @@ private void triggerReindexOfDefunctIndices() {
for (final File defunctIndex : defunctIndices) {
try {
- if (isLucene4IndexPresent(defunctIndex)) {
- logger.info("Encountered Lucene 8 index {} and also the corresponding Lucene 4 index; will only trigger rebuilding of one directory.", defunctIndex);
- rebuildCount.incrementAndGet();
- continue;
- }
Review Comment:
Thanks for confirming, removing the Lucene 4-related code certainly makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13186 corrected required runtime dep lucene-backward-codecs and … [nifi]
Posted by "joewitt (via GitHub)" <gi...@apache.org>.
joewitt commented on code in PR #8782:
URL: https://github.com/apache/nifi/pull/8782#discussion_r1595491134
##########
nifi-extension-bundles/nifi-provenance-repository-bundle/nifi-persistent-provenance-repository/src/main/java/org/apache/nifi/provenance/index/lucene/LuceneEventIndex.java:
##########
@@ -182,14 +182,7 @@ private void triggerReindexOfDefunctIndices() {
for (final File defunctIndex : defunctIndices) {
try {
- if (isLucene4IndexPresent(defunctIndex)) {
- logger.info("Encountered Lucene 8 index {} and also the corresponding Lucene 4 index; will only trigger rebuilding of one directory.", defunctIndex);
- rebuildCount.incrementAndGet();
- continue;
- }
Review Comment:
We don't actually support or need to support the Lucene4 style indexes anymore at all so I removed that. I kept the lucene4 example test files but renamed their structure to look like lucene8 style. In short - no we only have to deal with Lucene8 or greater style indexes/naming now. I read on Lucene's site the backward code only handles one major version change anyway (not sure if true/etc..) but either way we're long gone from Lucene4 anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13186 corrected required runtime dep lucene-backward-codecs and … [nifi]
Posted by "joewitt (via GitHub)" <gi...@apache.org>.
joewitt commented on PR #8782:
URL: https://github.com/apache/nifi/pull/8782#issuecomment-2101910023
Additional code changes applied now. Tested with a previous build generating Lucene-8 indexes with a range of content. Verified then again post this patch and it picks up where it left off, is still queryable, and continues generating new indexes with the lucene-9 naming. Was able to query both old and new data in the system. Also verifed query syntax still works despite not including the queryparsers module at this point.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13186 corrected required runtime dep lucene-backward-codecs and … [nifi]
Posted by "joewitt (via GitHub)" <gi...@apache.org>.
joewitt commented on code in PR #8782:
URL: https://github.com/apache/nifi/pull/8782#discussion_r1595488802
##########
nifi-extension-bundles/nifi-provenance-repository-bundle/pom.xml:
##########
@@ -46,16 +46,6 @@
<artifactId>lucene-core</artifactId>
<version>${lucene.version}</version>
</dependency>
- <dependency>
- <groupId>org.apache.lucene</groupId>
- <artifactId>lucene-analyzers-common</artifactId>
- <version>${lucene.version}</version>
- </dependency>
- <dependency>
- <groupId>org.apache.lucene</groupId>
- <artifactId>lucene-queryparser</artifactId>
- <version>${lucene.version}</version>
- </dependency>
Review Comment:
@exceptionfactory They used to be in this depMgmt section and actually dependened on the in the persistent prov repo. However, in the great dependency destruction JIRA of last month they were (plus lucene-backward-codecs) found to be in no apparent way used.
I was notified by another user that they upgraded from the pre Lucene9 to post Lucene9 and it broke lacking that lucene-backward-codec. So I restored that one as runtime scope. Then spent a good bit of time reading about these two. There are some SPI mechanisms in Lucene but no documentation or code linkage I could find suggested we use need these. I also checkwith Payne and he was suspected they used to be referenced and are not now.
So they're gone until we can show we actually need them. A full build of nifi without them appears to work well including searching with wildcards/etc...
Our integration tests don't actually exercise the backward code at this point. That was verified now manually.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org