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.

Sign up for GitHub

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

Jump to bottom

update ingest-attachment to use Tika 1.17 and newer deps #27824

Merged
merged 4 commits into from
Dec 15, 2017
Merged

update ingest-attachment to use Tika 1.17 and newer deps #27824

merged 4 commits into from
Dec 15, 2017

Conversation

talevy
Copy link
Contributor

@talevy talevy commented Dec 14, 2017

  • this pr updates tika and its dependencies
  • updates the SHAs
  • updates the class excludes

@talevy
update ingest-attachment to use Tika 1.16 and newer deps
82fcca0 
- this pr updates tika and its dependencies
- updates the SHAs
- updates the class excludes
@talevy talevy added :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP :Plugin Ingest Attachment >non-issue labels Dec 14, 2017
@talevy talevy requested a review from dadoonet December 14, 2017 18:30
jasontedor
jasontedor approved these changes Dec 14, 2017
View reviewed changes
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM. If it's green, ship it.

@dadoonet
Copy link
Member

1.17 has been released. Should we keep us up to date ?

@talevy
Copy link
Contributor Author

talevy commented Dec 14, 2017

@dadoonet woops. I saw it was released on their website, but then used mvnrepository.com to let me know if the sources were actually released, and they weren't there. I didn't dive deeper, that was my mistake. You're right. I will update to 1.17

as a side note... is there a sane way to manage the excluded classes list? I am using editor-magic to merge the exception list thrown by thirdPartyAudit and the existing list, then sorting them. Feels odd.

@dadoonet
Copy link
Member

About thirdPartyAudit, yeah this a lot of manual work. Didn’t find an easy way for doing it.

@talevy talevy changed the title update ingest-attachment to use Tika 1.16 and newer deps update ingest-attachment to use Tika 1.17 and newer deps Dec 14, 2017
@talevy
Copy link
Contributor Author

talevy commented Dec 14, 2017

@dadoonet luckily 1.17 doesn't change much between 1.16... updated to latest

jasontedor
jasontedor approved these changes Dec 14, 2017
View reviewed changes
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

Still LGTM.

@talevy
Copy link
Contributor Author

talevy commented Dec 15, 2017 •
edited
Loading

apologies for the extra back-and-forth @jasontedor. I thought I ran check, but I only ran precommit. 1.17 did actually change behavior. It throws an exception when reading empty files. It did not do this before 😄. I've modified the processor to preserve old behavior... tests were failing. go tests!

@jasontedor
Copy link
Member

Okay, I'll review this change.

jasontedor
jasontedor reviewed Dec 15, 2017
View reviewed changes
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

Any chance of a test for the zero-byte situation?

@talevy
Copy link
Contributor Author

talevy commented Dec 15, 2017

there is a test, that is what caught the change. it was failing.

@jasontedor
Copy link
Member

Sorry @talevy, I misunderstood. I found what you're referring to:

20:45:34 [jason:~/src/elastic/elasticsearch/plugins/ingest-attachment/src/test] master+ ± find . -size 0
./resources/org/elasticsearch/ingest/attachment/test/sample-files/text-empty.txt

I am good with this. Thanks.

jasontedor
jasontedor approved these changes Dec 15, 2017
View reviewed changes
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM. Third time is a charm!

@talevy talevy merged commit 43ff38c into elastic:master Dec 15, 2017
@talevy talevy deleted the update-tika branch December 15, 2017 21:47
talevy added a commit that referenced this pull request Dec 15, 2017
@talevy
update ingest-attachment to use Tika 1.17 and newer deps (#27824)
99cce58 
- this pr updates tika and its dependencies
- updates the SHAs
- updates the class excludes
@talevy talevy added the v6.1.1 label Dec 15, 2017
talevy added a commit that referenced this pull request Dec 15, 2017
@talevy
update ingest-attachment to use Tika 1.17 and newer deps (#27824)
471db4d 
- this pr updates tika and its dependencies
- updates the SHAs
- updates the class excludes
@jasontedor jasontedor mentioned this pull request Dec 20, 2017
Closed
spinscale added a commit to spinscale/elasticsearch that referenced this pull request Dec 22, 2017
@spinscale
Tests: Fix packaging tests after attachment plugin upgrade
b07041b 
The tests explicitely check for the version of some java dependencies
and thus needed to be fixed.

Relates elastic#27824
@spinscale spinscale mentioned this pull request Dec 22, 2017
Merged
spinscale added a commit that referenced this pull request Dec 22, 2017
@spinscale
Tests: Fix packaging tests after attachment plugin upgrade (#27959)
012ea03 
The tests explicitely check for the version of some java dependencies
and thus needed to be fixed.

Relates #27824

Closes #27916
spinscale added a commit that referenced this pull request Dec 22, 2017
@spinscale
Tests: Fix packaging tests after attachment plugin upgrade (#27959)
18d01d7 
The tests explicitely check for the version of some java dependencies
and thus needed to be fixed.

Relates #27824

Closes #27916
spinscale added a commit that referenced this pull request Dec 22, 2017
@spinscale
Tests: Fix packaging tests after attachment plugin upgrade (#27959)
8b132f0 
The tests explicitely check for the version of some java dependencies
and thus needed to be fixed.

Relates #27824

Closes #27916
spinscale added a commit that referenced this pull request Dec 22, 2017
@spinscale
Tests: Fix packaging tests after attachment plugin upgrade (#27959)
22e6b23 
The tests explicitely check for the version of some java dependencies
and thus needed to be fixed.

Relates #27824

Closes #27916
spinscale added a commit that referenced this pull request Dec 22, 2017
@spinscale
Tests: Fix packaging tests after attachment plugin upgrade (#27959)
0b2b2bc 
The tests explicitely check for the version of some java dependencies
and thus needed to be fixed.

Relates #27824

Closes #27916
tlrx pushed a commit that referenced this pull request Jan 2, 2018
@talevy @tlrx
update ingest-attachment to use Tika 1.17 and newer deps (#27824)
4a8f887 
- this pr updates tika and its dependencies
- updates the SHAs
- updates the class excludes
@tlrx tlrx added the v6.2.0 label Jan 2, 2018
@tlrx
Copy link
Member

tlrx commented Jan 2, 2018

I just backported this to 6.x in 4a8f887

@talevy
Copy link
Contributor Author

talevy commented Jan 3, 2018

thanks @tlrx!

martijnvg added a commit that referenced this pull request Jan 4, 2018
@martijnvg
Merge remote-tracking branch 'es/6.x' into ccr-6.x
564eee0 
* es/6.x: (48 commits)
  Bump compat version for local depdendent test to 6.2.0
  Pass `java.locale.providers=COMPAT` to Java 9 onwards (#28080)
  Allow shrinking of indices from a previous major (#28076)
  Add Writeable.Reader support to TransportResponseHandler (#28010)
  Fix cluster.routing.allocation.enable and cluster.routing.rebalance.enable casing (#28037)
  [Test] Fix scores for dcg in RankEvalRequestIT and RankEvalYamlIT
  [Docs] Add note on limitation for significant_text with nested objects (#28052)
  [Test] Fix allowed delta for calculated scores in DiscountedCumulativeGainTests
  Enable convert processor to support Long and Double. (#27957)
  Enable Wildfly tests on JDK 9 and JDK 10
  update ingest-attachment to use Tika 1.17 and newer deps (#27824)
  Only bind loopback addresses when binding to local
  Fix assertion in Wildfly build
  Fix typo in comment in Wildfly build
  Use ephemeral ports in Wildfly tests
  Update fuzzy-query.asciidoc (#28032)
  Add node id to shard failure message (#28024)
  Introduce limit to the number of terms in Terms Query (#27968)
  Upgrade Gradle Shadow plugin to 2.0.2
  Upgrade to JMH 1.19
  ...
@clintongormley clintongormley added :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP and removed :Plugin Ingest Attachment labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasontedor jasontedor jasontedor approved these changes

@dadoonet dadoonet Awaiting requested review from dadoonet

Assignees
No one assigned
Labels
:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >upgrade v5.6.6 v6.1.1 v6.2.0 v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@talevy @dadoonet @jasontedor @tlrx @clintongormley @colings86