Skip to content

Conversation

@imotov
Copy link
Contributor

@imotov imotov commented May 31, 2018

Adds a single JDBC driver jar that shades all external dependencies and bundles
all necessary classes.

Closes #29856

Adds a single JDBC driver jar that shades all external dependencies and bundles
all necessary classes.

Closes elastic#29856
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

It looks right to me but I think it is worth me downloading it and playing with it for a while which I'll try to do soon.

@imotov
Copy link
Contributor Author

imotov commented May 31, 2018

downloading it and playing with it

I would really appreciate that.

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

LGTM.

I've heard of issues with this in Intellij but @imotov uses intellij and didn't seem to have hit them.

I think it'd be nice to use the shaded jar in the qa tests but that this is worth merging as is. The qa tests will be enough work to fight and we have manually tested that the shaded jar works.

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.

I don't think we should use the default artifact name (jdbc); I think for now we should use an artifact name like x-pack-sql-jdbc and in the future elasticsearch-sql-jdbc but the time for latter is not now. You can do this by setting the archivesBaseName.

Additionally, I am curious why you dropped the use of nebula here? This means that the jdbc POM will not look like the rest of our POMs. For example, the SCM element in the POMs comes from the Nebula plugin (although it looks broken right now as it's being populated with an empty URL; that would be a follow-up).

@imotov
Copy link
Contributor Author

imotov commented Jun 6, 2018

@jasontedor I had to switch from nebula because it kept adding dependencies that were already included into the shadow jar. I will try to switch back to nebula and find a way to exclude dependencies.

@imotov
Copy link
Contributor Author

imotov commented Jun 6, 2018

@jasontedor I pushed some changes, could you take another look when you have a chance?

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.

Looks great; thanks for taking the extra iteration on this one.

@imotov imotov merged commit 01140a3 into elastic:master Jun 8, 2018
imotov added a commit that referenced this pull request Jun 8, 2018
Replaces zip archive containing multiple jars with a single JDBC driver jar 
that shades all external dependencies.

Closes #29856
dnhatn added a commit that referenced this pull request Jun 10, 2018
* 6.x:
  Move default location of dependencies report (#31228)
  Remove dependencies report task dependencies (#31227)
  Add recognition of MPL 2.0 (#31226)
  Fix unknown licenses (#31223)
  Fully encapsulate LocalCheckpointTracker inside of the engine (#31213)
  Remove version from license file name for GCS SDK (#31221)
  Remove DocumentFieldMappers#simpleMatchToFullName. (#31041)
  [DOCS] Removes 6.3.1 release notes
  [DOCS] Splits release notes by major version
  Remove DocumentFieldMappers#smartNameFieldMapper, as it is no longer needed. (#31018)
  Remove extraneous references to 'tokenized' in the mapper code. (#31010)
  SQL: Make a single JDBC driver jar (#31012)
  QA: Fix rolling restart tests some more
  Allow to trim all ops above a certain seq# with a term lower than X
  high level REST api: cancel task (#30745)
  Mute TokenBackwardsCompatibilityIT.testMixedCluster
  Mute WatchBackwardsCompatibilityIT.testWatcherRestart
  Enhance license detection for various licenses (#31198)
  [DOCS] Add note about long-lived idle connections (#30990)
  Add high-level client methods that accept RequestOptions (#31069)
  Remove RestGetAllMappingsAction (#31129)
  Move RestGetSettingsAction to RestToXContentListener (#31101)
  Move number of language analyzers to analysis-common module (#31143)
  flush job to ensure all results have been written (#31187)
dnhatn added a commit that referenced this pull request Jun 10, 2018
* master:
  Move default location of dependencies report (#31228)
  Remove dependencies report task dependencies (#31227)
  Add recognition of MPL 2.0 (#31226)
  Fix unknown licenses (#31223)
  Remove version from license file name for GCS SDK (#31221)
  Fully encapsulate LocalCheckpointTracker inside of the engine (#31213)
  [DOCS] Added 'fail_on_unsupported_field' param to MLT. Closes #28008 (#31160)
  Add licenses for transport-nio (#31218)
  Remove DocumentFieldMappers#simpleMatchToFullName. (#31041)
  Allow to trim all ops above a certain seq# with a term lower than X, post backport fix (#31211)
  Compliant SAML Response destination check (#31175)
  Remove DocumentFieldMappers#smartNameFieldMapper, as it is no longer needed. (#31018)
  Remove extraneous references to 'tokenized' in the mapper code. (#31010)
  Allow to trim all ops above a certain seq# with a term lower than X (#30176)
  SQL: Make a single JDBC driver jar (#31012)
  Enhance license detection for various licenses (#31198)
  [DOCS] Add note about long-lived idle connections (#30990)
  Move number of language analyzers to analysis-common module (#31143)
  Default max concurrent search req. numNodes * 5 (#31171)
  flush job to ensure all results have been written (#31187)
jasontedor added a commit to rjernst/elasticsearch that referenced this pull request Jun 10, 2018
…ecker

* elastic/master: (309 commits)
  [test] add fix for rare virtualbox error (elastic#31212)
  Move default location of dependencies report (elastic#31228)
  Remove dependencies report task dependencies (elastic#31227)
  Add recognition of MPL 2.0 (elastic#31226)
  Fix unknown licenses (elastic#31223)
  Remove version from license file name for GCS SDK (elastic#31221)
  Fully encapsulate LocalCheckpointTracker inside of the engine (elastic#31213)
  [DOCS] Added 'fail_on_unsupported_field' param to MLT. Closes elastic#28008 (elastic#31160)
  Add licenses for transport-nio (elastic#31218)
  Remove DocumentFieldMappers#simpleMatchToFullName. (elastic#31041)
  Allow to trim all ops above a certain seq# with a term lower than X, post backport fix (elastic#31211)
  Compliant SAML Response destination check (elastic#31175)
  Remove DocumentFieldMappers#smartNameFieldMapper, as it is no longer needed. (elastic#31018)
  Remove extraneous references to 'tokenized' in the mapper code. (elastic#31010)
  Allow to trim all ops above a certain seq# with a term lower than X (elastic#30176)
  SQL: Make a single JDBC driver jar (elastic#31012)
  Enhance license detection for various licenses (elastic#31198)
  [DOCS] Add note about long-lived idle connections (elastic#30990)
  Move number of language analyzers to analysis-common module (elastic#31143)
  Default max concurrent search req. numNodes * 5 (elastic#31171)
  ...
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
@imotov imotov deleted the issue-29856-single-jdbc-jar branch May 1, 2020 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants