Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BI-2156 - Upgrade Apache Tika to 2.3.0+ #432

Merged
merged 4 commits into from
Nov 20, 2024
Merged

BI-2156 - Upgrade Apache Tika to 2.3.0+ #432

merged 4 commits into from
Nov 20, 2024

Conversation

mlm483
Copy link
Contributor

@mlm483 mlm483 commented Nov 14, 2024

Description

Story: BI-2156

  • Upgraded apache tika to 2.9.2.
  • Removed as many hard-coded transitive dependencies as possible.
  • Hard-coded transitive dependency apache commons compress, because some tests were failing without specifying a compatible version.

Testing

You may need to do a maven clean install.

  • Test uploading files, verify no unexpected errors.
  • Ensure tests pass.

Checklist:

  • I have performed a self-review of my own code
  • I have tested my code and ensured it meets the acceptance criteria of the story
  • I have tested that my code works with both the brapi-java-server and BreedBase
  • I have create/modified unit tests to cover this change
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to documentation
  • I have run TAF: <please include a link to TAF run>

also removed hard-coded transitive dependencies
@github-actions github-actions bot added the bug Something isn't working label Nov 14, 2024
@mlm483 mlm483 requested review from a team, davedrp and nickpalladino and removed request for a team November 15, 2024 15:09
@mlm483 mlm483 changed the title BI-2156 - BI-2156 Upgrade Apache Tika to 2.3.0+ BI-2156 - Upgrade Apache Tika to 2.3.0+ Nov 15, 2024
Copy link
Member

@nickpalladino nickpalladino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was able to import traits without error

pom.xml Outdated
@@ -91,10 +91,9 @@
<mockito.version>4.3.1</mockito.version>
<brapi-java-client.version>2.1-SNAPSHOT</brapi-java-client.version>
<commons-io.version>2.11.0</commons-io.version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the commons-io.version needed?

Copy link
Contributor Author

@mlm483 mlm483 Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was unnecessary, I removed it. Thanks.

@@ -378,36 +377,16 @@
<artifactId>brapi-java-client</artifactId>
<version>${brapi-java-client.version}</version>
</dependency>
<dependency>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume tika is pulling all these in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, maven pulls those in to build tika as they are dependencies of tika. I don't know how common or advisable it is to hard-code transitive dependencies with maven projects, I used trial and error to determine that maven did the right thing for all the transitive dependencies besides apache commons compress, which I hard-coded. I think the issue might have been that two primary dependencies of bi-api both depend on apache commons compress, and maven was defaulting to using a version that didn't work with tika.

Copy link
Contributor

@davedrp davedrp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passed Test

@mlm483 mlm483 merged commit 16cdff9 into develop Nov 20, 2024
1 check passed
@mlm483 mlm483 deleted the bug/BI-2156 branch November 20, 2024 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants