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 MultiTypeMappingTransformation and test #1189

Merged
merged 24 commits into from
Dec 11, 2024

Conversation

AndreKurait
Copy link
Member

@AndreKurait AndreKurait commented Dec 10, 2024

Description

Support Union resolution of MultiTypeMappingTransformation in metadata migrations.

Adds argument --multi-type-behavior allowing customers to specify UNION and migrate multi type indices.

  • Category: New feature
  • Why these changes are required? 6.8 multi type mapping support
  • What is the old behavior before changes and new behavior after changes? Threw unsupported if index had multiple types

Issues Resolved

[List any issues this PR will resolve]

Is this a backport? If so, please add backport PR # and/or commits #

Testing

Added E2E unit test

Check List

  • New functionality includes testing
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented
  • 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.

peternied and others added 2 commits December 9, 2024 18:38
After recently reviewing metadata migration end to end test cases I
noticed there was 56 different tests, this was a large increase, taking
nearly 20 minutes of runtime.  Reviewing the parameters of the tests
I'm running evaluate and then immediately following migrate, this
should reduce the test cases by nearly half and increase of coverage
using http and snapshot sources.

I have tweaked how we use templates moving back to the logic of
legacy templates for ES 6.X and using both index and index compontent
templates for future ES versions, which should be another 30%
redunction.

Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Andre Kurait <[email protected]>
Signed-off-by: Andre Kurait <[email protected]>
Signed-off-by: Andre Kurait <[email protected]>
Signed-off-by: Andre Kurait <[email protected]>
@@ -1,50 +1,33 @@
package org.opensearch.migrations;
Copy link
Member

Choose a reason for hiding this comment

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

Since there is a decent amount of refactoring going on here want to pull #1186 into this change, it should help your cycles iterate more quickly

Signed-off-by: Andre Kurait <[email protected]>
@AndreKurait AndreKurait merged commit fdf32c2 into opensearch-project:main Dec 11, 2024
20 checks passed
Copy link

codecov bot commented Dec 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.57%. Comparing base (6d3b225) to head (d276695).
Report is 25 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1189      +/-   ##
============================================
+ Coverage     80.55%   80.57%   +0.02%     
- Complexity     3074     3081       +7     
============================================
  Files           421      421              
  Lines         15545    15592      +47     
  Branches       1047     1053       +6     
============================================
+ Hits          12522    12564      +42     
- Misses         2380     2386       +6     
+ Partials        643      642       -1     
Flag Coverage Δ
unittests 80.57% <ø> (+0.02%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -111,6 +111,44 @@ jobs:
with:
gradle-version: ${{ env.gradle-version }}
gradle-home-cache-cleanup: true
- name: Generate Cache Key from Dockerfiles
Copy link
Member

Choose a reason for hiding this comment

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

@AndreKurait Can you revert the change to this file or maybe we can sync on the utility of this change?

The additional workflow complexity + extra effort to remember to sync these lists between java source and CI.yml gives me pause about including this change. It looks like this was done in an effort to speed up and issues with the test case that ended up being disabled due to an unrelated root cause.

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.

2 participants