Replace checked-in ZIP with a dynamic dependency #514
Conversation
Codecov Report
@@ Coverage Diff @@
## main #514 +/- ##
=========================================
Coverage 95.24% 95.24%
Complexity 2744 2744
=========================================
Files 276 276
Lines 7410 7410
Branches 537 537
=========================================
Hits 7058 7058
Misses 298 298
Partials 54 54
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
Signed-off-by: penghuo <penghuo@gmail.com>
| @@ -188,6 +189,8 @@ task compileJdbc(type: Exec) { | |||
| String bwcVersion = "1.13.2.0"; | |||
| String baseName = "sqlBwcCluster" | |||
| String bwcFilePath = "src/test/resources/bwc/" | |||
| String bwcOpenDistroPlugin = "opendistro-sql-1.13.2.0.zip" | |||
| String bwcRemoteFile = 'https://d3g5vo6xdbdb9a.cloudfront.net/downloads/elasticsearch-plugins/opendistro-sql/opendistro-sql-1.13.2.0.zip' | |||
There was a problem hiding this comment.
How do we maintain this zip file in cloudfront? Like if this package has merged a new bug fix commit, do we need to build the zip file locally then upload it to cloudfront? Then although it's not a checked-in dependency anymore, it's still not a dynamic dependency?
There was a problem hiding this comment.
opendistro-sql-1.13.2.0.zip is the released latest opendistro SQL plugin. we didn't maintain opendistro-sql-1.13.2.0.zip
There was a problem hiding this comment.
if opendistro SQL plugin release a new version, do we need to replace this zip file on cloudfront with a one build with latest version?
There was a problem hiding this comment.
yes, it is required.
There was a problem hiding this comment.
What's the official URL for this .zip? I see in https://opendistro.github.io/for-elasticsearch-docs/docs/install/plugins/ that we reference the cloudfront URL. If that's how it should be, so be it, but I would use something like opendistrowhateverthatwas.org if that exists.
| @@ -188,6 +189,8 @@ task compileJdbc(type: Exec) { | |||
| String bwcVersion = "1.13.2.0"; | |||
| String baseName = "sqlBwcCluster" | |||
| String bwcFilePath = "src/test/resources/bwc/" | |||
| String bwcOpenDistroPlugin = "opendistro-sql-1.13.2.0.zip" | |||
There was a problem hiding this comment.
Nit: I would use bcwVersion from above in the file name.
| @@ -188,6 +189,8 @@ task compileJdbc(type: Exec) { | |||
| String bwcVersion = "1.13.2.0"; | |||
| String baseName = "sqlBwcCluster" | |||
| String bwcFilePath = "src/test/resources/bwc/" | |||
| String bwcOpenDistroPlugin = "opendistro-sql-1.13.2.0.zip" | |||
| String bwcRemoteFile = 'https://d3g5vo6xdbdb9a.cloudfront.net/downloads/elasticsearch-plugins/opendistro-sql/opendistro-sql-1.13.2.0.zip' | |||
There was a problem hiding this comment.
What's the official URL for this .zip? I see in https://opendistro.github.io/for-elasticsearch-docs/docs/install/plugins/ that we reference the cloudfront URL. If that's how it should be, so be it, but I would use something like opendistrowhateverthatwas.org if that exists.
|
@dblock I think plugin zip only available at cloudfront URL. I didn't aware opendistrowhateverthatwas.org link. |
Description
Replace checked-in ZIP with a dynamic dependency
Issues Resolved
#491
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.