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

[Extensions] changed version from 3.0.0 to version.current in extensionManagerTest #7176

Conversation

varuntumbe
Copy link
Contributor

…s.java

Description

changed version from 3.0.0 to version.current in extensionManagerTest

Issues Resolved

#7051

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
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

@varuntumbe varuntumbe force-pushed the Feature.change_version.7051 branch from bb8b25d to c76f700 Compare April 15, 2023 11:18
@varuntumbe
Copy link
Contributor Author

Hi @owaiskazi19 ,

I have changed the opensearchVersion and minimumCompatibleVersion but have not modified version of dependency as it has not been modifed in 2.x, If you want that I can modify that as well.

Thanks and Regards
Varun

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

codecov-commenter commented Apr 15, 2023

Codecov Report

Merging #7176 (bb8b25d) into main (3cb2565) will increase coverage by 0.13%.
Report is 735 commits behind head on main.
The diff coverage is 65.54%.

❗ Current head bb8b25d differs from pull request most recent head c76f700. Consider uploading reports for the commit c76f700 to get more accurate results

@@             Coverage Diff              @@
##               main    #7176      +/-   ##
============================================
+ Coverage     70.67%   70.81%   +0.13%     
- Complexity    59457    59528      +71     
============================================
  Files          4852     4848       -4     
  Lines        285189   285158      -31     
  Branches      41118    41111       -7     
============================================
+ Hits         201564   201930     +366     
+ Misses        67042    66640     -402     
- Partials      16583    16588       +5     
Files Changed Coverage Δ
...ark/store/remote/filecache/FileCacheBenchmark.java 0.00% <0.00%> (ø)
...ch/index/codec/customcodecs/CustomCodecPlugin.java 0.00% <ø> (ø)
...customcodecs/Lucene95CustomStoredFieldsFormat.java 25.00% <ø> (ø)
...opensearch/index/codec/customcodecs/ZstdCodec.java 80.00% <ø> (ø)
.../index/codec/customcodecs/ZstdCompressionMode.java 84.09% <ø> (ø)
...arch/index/codec/customcodecs/ZstdNoDictCodec.java 80.00% <ø> (ø)
.../codec/customcodecs/ZstdNoDictCompressionMode.java 76.71% <ø> (ø)
...rch/action/admin/cluster/node/stats/NodeStats.java 52.43% <0.00%> (+0.60%) ⬆️
...org/opensearch/cluster/routing/RecoverySource.java 71.11% <0.00%> (-1.49%) ⬇️
...rg/opensearch/common/settings/ClusterSettings.java 92.85% <ø> (-0.33%) ⬇️
... and 15 more

... and 476 files with indirect coverage changes

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Before we merge, don't we want a range here?

" opensearchVersion: '3.0.0'",
" minimumCompatibleVersion: '3.0.0'",
" opensearchVersion: '" + Version.CURRENT.toString() + "'",
" minimumCompatibleVersion: '" + Version.CURRENT.toString() + "'",
Copy link
Member

Choose a reason for hiding this comment

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

I think for tests we want to use the minimum as the major.0.0, otherwise this perpetuates the idea that the extension is narrowly compatible with the current version only, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @dblock ,

Are you saying that we should we have different version like 2.0.0 for first extension and 3.0.0 or 4.0.0 for second extension and so on ? meaning tests has to pass for extensions of all version otherwise we are saying tests are compatible for only the current versions and not the ones before ?

Thanks and Regards
Varun

Copy link
Member

Choose a reason for hiding this comment

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

Exactly. Have predictable examples plus one for current version.

@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity. Remove stalled label or comment or this will be closed in 7 days.

@opensearch-trigger-bot opensearch-trigger-bot bot added the stalled Issues that have stalled label Jun 22, 2023
@opensearch-trigger-bot
Copy link
Contributor

This PR was closed because it has been stalled for 7 days with no activity.

@kotwanikunal
Copy link
Member

Apologies. This PR was auto closed without reaching a resolution from the maintainers.
Re-opening to move it forward.
Thanks for your contributions to OpenSearch!

@github-actions
Copy link
Contributor

Compatibility status:

Checks if related components are compatible with change c76f700

Incompatible components

Skipped components

Compatible components

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@ashking94
Copy link
Member

@varuntumbe - Do you think if you will be coming up with a revision on this? If not, please close it.

@varuntumbe varuntumbe closed this Dec 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog stalled Issues that have stalled
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants