-
Notifications
You must be signed in to change notification settings - Fork 75
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
removing job-scheduler zip and replacing with distribution build #487
Conversation
Signed-off-by: Amit Galitzky <[email protected]>
build.gradle
Outdated
project.mkdir js_resource_folder | ||
ant.get(src: 'https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/' | ||
+ opensearch_no_snapshot | ||
+ '/latest/linux/x64/builds/opensearch/plugins/opensearch-job-scheduler-' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we use hard code x64
here, not sure if it will impact CI/IT workflow for other platform, @peterzhuamazon any suggestion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since github actions offer default runner are x64 I see no problem running on hardcoded x64.
Jenkins are testing both x64 and arm64 tho after each build.
If you really want arm64 to be on github action you need to manage self-hosted runners yourself tho.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will eventually become a problem, but not just because of x64. For example, windows. For now this is fine.
build.gradle
Outdated
} | ||
if (isSnapshot) { | ||
opensearch_build += "-SNAPSHOT" | ||
} | ||
opensearch_no_snapshot = opensearch_version.substring(0, opensearch_version.indexOf("-", opensearch_version.indexOf("-") + 1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is just to remove the -SNAPSHOT
, can't we just persist the opensearch_build
var as a separate var before the optional -SNAPSHOT
is added on lines 30-32?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, what's the difference between job_scheduler_no_snapshot
and opensearch_no_snapshot
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just replace -SNAPSHOT
with
should be enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
job_scheduler_no_snapshot = 2.0.0.0-alpha1
opensearch_no_snapshot = 2.0.0-alpha1
The optional -SNAPSHOT is added after adding the .0
@peterzhuamazon good call, I'll change to opensearch_version.replace("-SNAPSHOT, "")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ohltyler since one has the .0 and one doesn't, I changed it to .replace
from the opensearch_version
. Might have misunderstood what you meant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, missed that. Your change looks good to me!
Codecov Report
@@ Coverage Diff @@
## main #487 +/- ##
============================================
+ Coverage 77.96% 78.24% +0.27%
- Complexity 4145 4167 +22
============================================
Files 296 296
Lines 17652 17652
Branches 1877 1877
============================================
+ Hits 13762 13811 +49
+ Misses 2995 2950 -45
+ Partials 895 891 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Signed-off-by: Amit Galitzky <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Signed-off-by: Amit Galitzky [email protected]
Description
Remove job-scheduler zip and instead download distribution from ci.opensearch.org and using that for running integCluster. This will later be replaced with a maven download when zips are uploaded to maven.
Issues Resolved
#17
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.