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

MIGRATIONS-1200 - Migrations Repo Cleanup #263

Merged
merged 16 commits into from
Aug 17, 2023

Conversation

sumobrian
Copy link
Collaborator

@sumobrian sumobrian commented Aug 15, 2023

Description

[Describe what this change achieves]

  • Category - Code cleanup before release branch is cut
  • Why these changes are required? The files removed in this PR are not needed for release. The items renamed we renamed to provide clarity.
  • What is the old behavior before changes and new behavior after changes? There are no behavior changes.

Once this PR is approved, a release branch will be cut. On this release branch the additional changes outlined in MIGRATIONS-1200 will addressed.

Issues Resolved

[List any issues this PR will resolve]
https://opensearch.atlassian.net/browse/MIGRATIONS-1200

Testing

[Please provide details of testing done: unit testing, integration testing and manual testing]

Check List

  • Verified end-2-end solution still works.

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.

@sumobrian sumobrian added the draft Indicate that a PR is in draft mode and not ready to merged label Aug 15, 2023
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #263 (d0c10a4) into main (2c8e3ab) will increase coverage by 8.16%.
Report is 1 commits behind head on main.
The diff coverage is 30.43%.

@@             Coverage Diff              @@
##               main     #263      +/-   ##
============================================
+ Coverage     61.65%   69.81%   +8.16%     
- Complexity      589      593       +4     
============================================
  Files            73       99      +26     
  Lines          2968     4145    +1177     
  Branches        275      401     +126     
============================================
+ Hits           1830     2894    +1064     
- Misses          970     1068      +98     
- Partials        168      183      +15     
Flag Coverage Δ
unittests 61.43% <30.43%> (-0.22%) ⬇️

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

Files Changed Coverage Δ
.../opensearch/migrations/replay/TrafficReplayer.java 8.57% <0.00%> (-0.48%) ⬇️
...on_core/cluster_migration_core/clients/__init__.py 80.00% <ø> (ø)
...cluster_migration_core/clients/rest_client_base.py 71.11% <ø> (ø)
...ster_migration_core/clients/rest_client_default.py 78.94% <ø> (ø)
...on_core/cluster_migration_core/clients/rest_ops.py 84.48% <ø> (ø)
...uster_migration_core/cluster_management/cluster.py 92.48% <ø> (ø)
...gration_core/cluster_management/cluster_objects.py 85.71% <ø> (ø)
...core/cluster_management/container_configuration.py 94.11% <ø> (ø)
...tion_core/cluster_management/docker_command_gen.py 100.00% <ø> (ø)
...core/cluster_management/docker_framework_client.py 90.37% <ø> (ø)
... and 17 more

@sumobrian sumobrian self-assigned this Aug 16, 2023
@sumobrian sumobrian removed the draft Indicate that a PR is in draft mode and not ready to merged label Aug 16, 2023
@sumobrian sumobrian changed the title [DRAFT] MIGRATIONS-1200 - Remove unused TrafficReplayer directory MIGRATIONS-1200 - Remove unused TrafficReplayer directory Aug 16, 2023
@sumobrian sumobrian changed the title MIGRATIONS-1200 - Remove unused TrafficReplayer directory MIGRATIONS-1200 - Migrations Repo Cleanup Aug 17, 2023
@lewijacn
Copy link
Collaborator

I pictured this as being

├── experimental
│   ├── upgrades
│   └── cluster_migration_core
│   └── cluster_traffic_capture
│   └── knowledge_base

The one thing that is really throwing me off is that we don't have the top level upgrades directory anymore

@sumobrian
Copy link
Collaborator Author

I pictured this as being

├── experimental
│   ├── upgrades
│   └── cluster_migration_core
│   └── cluster_traffic_capture
│   └── knowledge_base

The one thing that is really throwing me off is that we don't have the top level upgrades directory anymore

This is a good suggestion. I'm going to incorporate these changes.

@@ -30,8 +30,8 @@ jobs:
python -m pip install -r requirements.txt coverage pytest-cov
- name: Run Tests with Coverage
run: |
python -m pytest unit_tests/ --cov=cluster_migration_core --cov-report=xml --cov-branch
python -m pytest unit_tests/ --cov=experimental/cluster_migration_core --cov-report=xml --cov-branch
Copy link
Collaborator

Choose a reason for hiding this comment

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

--cov=cluster_migration_core as this is using the working-directory above

files: experimental/cluster_migration_core/coverage.xml
Copy link
Collaborator

Choose a reason for hiding this comment

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

This I am unsure about but might need to be cluster_migration_core/coverage.xml

@lewijacn
Copy link
Collaborator

org/opensearch/migrations/trafficcapture/proxyserver/netty/ExpiringSubstitutableItemPoolTest.java is seeming like a flaky test causing the gradle test to fail, need further RCA

@sumobrian sumobrian merged commit bc0eab1 into opensearch-project:main Aug 17, 2023
6 of 7 checks passed
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.

3 participants