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

Add windows CI 2.x #704

Closed
wants to merge 5 commits into from

Conversation

amitgalitz
Copy link
Member

@amitgalitz amitgalitz commented Oct 27, 2022

Signed-off-by: Amit Galitzky [email protected]

Description

Adds windows CI for 2.x branch

  1. Changed the usage of getPath() to first use .toURI() in some of the test cases. This is because just getting the path like we used to was returning a string like this: \C:\Sample\sample.txt. This isn't supported on Windows. Converting with toUri() works on all platforms.

Issues Resolved

#274

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: Amit Galitzky <[email protected]>
@amitgalitz
Copy link
Member Author

3 failings tests in windows:

- org.opensearch.ad.ml.CheckpointDaoTests.testDeserializeRCFModelPreINIT
 - org.opensearch.ad.ml.CheckpointDaoTests.testDeserializeRCFModelPostINIT
 - org.opensearch.ad.ml.CheckpointDaoTests.testDeserializeTRCFModel

Signed-off-by: Amit Galitzky <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Oct 27, 2022

Codecov Report

Merging #704 (38dfbaa) into 2.x (038fc54) will decrease coverage by 0.19%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x     #704      +/-   ##
============================================
- Coverage     79.24%   79.04%   -0.20%     
+ Complexity     4257     4239      -18     
============================================
  Files           301      301              
  Lines         17872    17870       -2     
  Branches       1897     1897              
============================================
- Hits          14163    14126      -37     
- Misses         2809     2838      +29     
- Partials        900      906       +6     
Flag Coverage Δ
plugin 79.04% <ø> (-0.20%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...java/org/opensearch/ad/task/ADBatchTaskRunner.java 82.37% <0.00%> (-2.13%) ⬇️
...ava/org/opensearch/ad/task/ADHCBatchTaskCache.java 90.12% <0.00%> (-1.24%) ⬇️
...ain/java/org/opensearch/ad/task/ADTaskManager.java 76.90% <0.00%> (-0.99%) ⬇️
...rch/ad/transport/AnomalyResultTransportAction.java 79.95% <0.00%> (-0.93%) ⬇️
.../main/java/org/opensearch/ad/ml/CheckpointDao.java 69.74% <0.00%> (-0.64%) ⬇️
...opensearch/ad/indices/AnomalyDetectionIndices.java 72.12% <0.00%> (-0.19%) ⬇️
...c/main/java/org/opensearch/ad/util/ParseUtils.java 77.77% <0.00%> (-0.08%) ⬇️
...ava/org/opensearch/ad/model/TimeConfiguration.java 100.00% <0.00%> (ø)

@amitgalitz amitgalitz marked this pull request as ready for review October 27, 2022 20:10
@amitgalitz amitgalitz requested a review from a team October 27, 2022 20:10
Signed-off-by: Amit Galitzky <[email protected]>
.github/workflows/CI.yml Outdated Show resolved Hide resolved
ohltyler
ohltyler previously approved these changes Oct 27, 2022
Copy link
Member

@ohltyler ohltyler left a comment

Choose a reason for hiding this comment

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

One minor comment. LGTM!

Signed-off-by: Amit Galitzky <[email protected]>
@amitgalitz amitgalitz added backport 2.4 infra Changes to infrastructure, testing, CI/CD, pipelines, etc. labels Oct 27, 2022
@ohltyler
Copy link
Member

Btw, do you know if we want to support for 3.0 as well? Wondering if it's worth front-porting there, and if it's not ready, could just add the changes, but have them commented out in the workflow file for now until that's possible.

@amitgalitz
Copy link
Member Author

Btw, do you know if we want to support for 3.0 as well? Wondering if it's worth front-porting there, and if it's not ready, could just add the changes, but have them commented out in the workflow file for now until that's possible.

Actually now that I check 3.0 has just been fixed. I can either forward this or close this. merge into main and backport to 2.x

@ohltyler
Copy link
Member

Btw, do you know if we want to support for 3.0 as well? Wondering if it's worth front-porting there, and if it's not ready, could just add the changes, but have them commented out in the workflow file for now until that's possible.

Actually now that I check 3.0 has just been fixed. I can either forward this or close this. merge into main and backport to 2.x

Front porting should be fine.

@amitgalitz amitgalitz closed this Oct 28, 2022
@amitgalitz
Copy link
Member Author

Btw, do you know if we want to support for 3.0 as well? Wondering if it's worth front-porting there, and if it's not ready, could just add the changes, but have them commented out in the workflow file for now until that's possible.

Actually now that I check 3.0 has just been fixed. I can either forward this or close this. merge into main and backport to 2.x

Front porting should be fine.

missed this haha, I guess we will merge the other one already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.4 infra Changes to infrastructure, testing, CI/CD, pipelines, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants