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.

Sign up for GitHub

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

Jump to bottom

Fix BWC test suite #1426

Merged
merged 4 commits into from
Oct 4, 2023
Merged

Fix BWC test suite #1426

merged 4 commits into from
Oct 4, 2023

Conversation

b4sjoo
Copy link
Collaborator

@b4sjoo b4sjoo commented Oct 3, 2023 •
edited
Loading

Description

This PR should unblock BWC test. Now we use regex to judge if OS version is updated or not.

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • 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.

@b4sjoo
Fix BWC test suite
4e57459 
Signed-off-by: Sicheng Song <[email protected]>
@b4sjoo b4sjoo had a problem deploying to ml-commons-cicd-env October 3, 2023 23:06 — with GitHub Actions Error
@b4sjoo b4sjoo had a problem deploying to ml-commons-cicd-env October 3, 2023 23:06 — with GitHub Actions Error
@b4sjoo b4sjoo had a problem deploying to ml-commons-cicd-env October 3, 2023 23:06 — with GitHub Actions Failure
@b4sjoo b4sjoo had a problem deploying to ml-commons-cicd-env October 3, 2023 23:06 — with GitHub Actions Failure
@b4sjoo b4sjoo marked this pull request as ready for review October 3, 2023 23:06
@b4sjoo
Debugging BWC
ea2ce80 
Signed-off-by: Sicheng Song <[email protected]>
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 3, 2023 23:39 — with GitHub Actions Inactive
@b4sjoo b4sjoo had a problem deploying to ml-commons-cicd-env October 3, 2023 23:39 — with GitHub Actions Failure
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 3, 2023 23:39 — with GitHub Actions Inactive
@b4sjoo b4sjoo had a problem deploying to ml-commons-cicd-env October 3, 2023 23:39 — with GitHub Actions Error
@codecov
Copy link

codecov bot commented Oct 3, 2023 •
edited
Loading

Codecov Report

Merging #1426 (df8bc47) into 2.x (d8c1162) will increase coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##                2.x    #1426      +/-   ##
============================================
+ Coverage     78.36%   78.37%   +0.01%     
+ Complexity     2345     2344       -1     
============================================
  Files           195      195              
  Lines          9529     9529              
  Branches        944      944              
============================================
+ Hits           7467     7468       +1     
+ Misses         1635     1634       -1     
  Partials        427      427
Flag Coverage Δ
ml-commons 78.37% <ø> (+0.01%) ⬆️

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

see 2 files with indirect coverage changes

@b4sjoo
Fix regex expression
79e45f2 
Signed-off-by: Sicheng Song <[email protected]>
@b4sjoo b4sjoo had a problem deploying to ml-commons-cicd-env October 4, 2023 01:05 — with GitHub Actions Error
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 4, 2023 01:05 — with GitHub Actions Inactive
@b4sjoo b4sjoo had a problem deploying to ml-commons-cicd-env October 4, 2023 01:05 — with GitHub Actions Failure
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 4, 2023 01:05 — with GitHub Actions Inactive
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 4, 2023 01:26 — with GitHub Actions Inactive
@b4sjoo b4sjoo had a problem deploying to ml-commons-cicd-env October 4, 2023 01:26 — with GitHub Actions Failure
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 4, 2023 01:26 — with GitHub Actions Inactive
@b4sjoo b4sjoo had a problem deploying to ml-commons-cicd-env October 4, 2023 01:26 — with GitHub Actions Error
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 4, 2023 01:45 — with GitHub Actions Inactive
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 4, 2023 01:45 — with GitHub Actions Inactive
@b4sjoo b4sjoo had a problem deploying to ml-commons-cicd-env October 4, 2023 01:45 — with GitHub Actions Failure
@b4sjoo b4sjoo had a problem deploying to ml-commons-cicd-env October 4, 2023 01:45 — with GitHub Actions Error
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 4, 2023 02:17 — with GitHub Actions Inactive
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 4, 2023 02:17 — with GitHub Actions Inactive
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 4, 2023 02:17 — with GitHub Actions Inactive
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 4, 2023 02:17 — with GitHub Actions Inactive
rbhavna
rbhavna previously approved these changes Oct 4, 2023
View reviewed changes
ylwu-amzn
ylwu-amzn reviewed Oct 4, 2023
View reviewed changes
plugin/src/test/java/org/opensearch/ml/bwc/MLCommonsBackwardsCompatibilityIT.java Outdated
osVersionArrayList.add(Integer.parseInt(matcher.group()));
}
if (osVersionArrayList.size() >= 2) {
return (osVersionArrayList.get(0) > 2 || osVersionArrayList.get(1) > 4);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for fixing this. Can you add some comments for these magic numbers: 2, 4? Also add some example osVersion to explain ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added.

@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 4, 2023 17:48 — with GitHub Actions Inactive
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 4, 2023 17:48 — with GitHub Actions Inactive
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 4, 2023 17:48 — with GitHub Actions Inactive
@b4sjoo b4sjoo temporarily deployed to ml-commons-cicd-env October 4, 2023 17:48 — with GitHub Actions Inactive
@b4sjoo b4sjoo merged commit ab216d5 into opensearch-project:2.x Oct 4, 2023
9 checks passed
@b4sjoo b4sjoo deleted the 2.x_bwcfix branch October 4, 2023 19:13
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 19, 2024
@b4sjoo @github-actions
Fix BWC test suite (#1426)
35f5105 
* Fix BWC test suite

Signed-off-by: Sicheng Song <[email protected]>

* Debugging BWC

Signed-off-by: Sicheng Song <[email protected]>

* Fix regex expression

Signed-off-by: Sicheng Song <[email protected]>

* Add some doc and examples to explain how fix works

Signed-off-by: Sicheng Song <[email protected]>

---------

Signed-off-by: Sicheng Song <[email protected]>
(cherry picked from commit ab216d5)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Feb 19, 2024
Merged
b4sjoo added a commit that referenced this pull request Feb 19, 2024
@opensearch-trigger-bot @b4sjoo
Fix BWC test suite (#1426) (#2135)
403798e 
* Fix BWC test suite

Signed-off-by: Sicheng Song <[email protected]>

* Debugging BWC

Signed-off-by: Sicheng Song <[email protected]>

* Fix regex expression

Signed-off-by: Sicheng Song <[email protected]>

* Add some doc and examples to explain how fix works

Signed-off-by: Sicheng Song <[email protected]>

---------

Signed-off-by: Sicheng Song <[email protected]>
(cherry picked from commit ab216d5)

Co-authored-by: Sicheng Song <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ylwu-amzn ylwu-amzn ylwu-amzn left review comments

@rbhavna rbhavna rbhavna approved these changes

@dhrubo-os dhrubo-os dhrubo-os approved these changes

Assignees
No one assigned
Labels
backport 2.10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@b4sjoo @ylwu-amzn @dhrubo-os @rbhavna