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

Replace checked-in ZIPs with dynamic dependencies #327

Merged
merged 2 commits into from
Apr 14, 2022

Conversation

downsrob
Copy link
Contributor

@downsrob downsrob commented Apr 12, 2022

Signed-off-by: Clay Downs downsrob@amazon.com

Issue #, if available:
#305

Description of changes:
Replace checked-in ZIPs with dynamic dependencies. The current job scheduler zip used for integration tests is downloaded from ci.opensearch.org. The backwards compatibility test job scheduler and index management versions are downloaded from https://github.com/opendistro-for-elasticsearch artifacts. These dependencies are downloaded during the gradle configuration phase, and will be refreshed with any task run.

Removes the mustache zip entirely, as I couldn't find any reference to it being used.

CheckList:

  • Commits are signed per the DCO using --signoff

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.

Signed-off-by: Clay Downs <downsrob@amazon.com>
@downsrob downsrob changed the title Removes zip dependencies Replace checked-in ZIPs with dynamic dependencies Apr 12, 2022
Signed-off-by: Clay Downs <downsrob@amazon.com>
@codecov-commenter
Copy link

codecov-commenter commented Apr 12, 2022

Codecov Report

Merging #327 (b97ccb6) into main (34b480c) will decrease coverage by 0.06%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main     #327      +/-   ##
============================================
- Coverage     76.71%   76.64%   -0.07%     
+ Complexity     2044     2043       -1     
============================================
  Files           253      253              
  Lines         11636    11636              
  Branches       1807     1807              
============================================
- Hits           8926     8918       -8     
- Misses         1742     1746       +4     
- Partials        968      972       +4     
Impacted Files Coverage Δ
...anagement/indexstatemanagement/model/Transition.kt 61.64% <0.00%> (-4.11%) ⬇️
.../rollup/action/start/TransportStartRollupAction.kt 72.94% <0.00%> (-2.36%) ⬇️
.../action/explain/TransportExplainTransformAction.kt 70.78% <0.00%> (-2.25%) ⬇️
...ndexstatemanagement/IndexStateManagementHistory.kt 78.62% <0.00%> (-1.38%) ⬇️
.../opensearch/indexmanagement/rollup/model/Rollup.kt 86.04% <0.00%> (-0.47%) ⬇️
...earch/indexmanagement/transform/model/Transform.kt 85.47% <0.00%> (ø)
...arch/indexmanagement/rollup/RollupSearchService.kt 64.81% <0.00%> (+3.70%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34b480c...b97ccb6. Read the comment docs.

@downsrob downsrob marked this pull request as ready for review April 12, 2022 18:47
@downsrob downsrob requested review from a team, bowenlan-amzn, thalurur and dblock April 12, 2022 18:47
}

// Download the job scheduler test dependency
getPluginResource(job_scheduler_resource_folder, job_scheduler_build_download)
Copy link
Contributor

@thalurur thalurur Apr 14, 2022

Choose a reason for hiding this comment

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

does this mean this method will be called for all gradle tasks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right. I believe that we also initialize the integTest clusters with the plugin zips for all targets in the configuration phase, so to get around downloading the resource always, we would need to refactor the cluster setup, which I didn't have time to take on at this point

Copy link
Contributor

Choose a reason for hiding this comment

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

got it, lets create an issue for this - we don't need to address it immediately

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@downsrob downsrob merged commit 6259f9a into opensearch-project:main Apr 14, 2022
@downsrob downsrob deleted the removes-zip branch April 14, 2022 18:18
wuychn pushed a commit to ochprince/index-management that referenced this pull request Mar 16, 2023
…#327)

* Removes zip dependencies

Signed-off-by: Clay Downs <downsrob@amazon.com>

* Removes mustache zip

Signed-off-by: Clay Downs <downsrob@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants