-
Notifications
You must be signed in to change notification settings - Fork 277
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 rpm support for integTest framework #1992
Add rpm support for integTest framework #1992
Conversation
Signed-off-by: Peter Zhu <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #1992 +/- ##
=============================================
+ Coverage 94.41% 100.00% +5.58%
=============================================
Files 187 6 -181
Lines 3739 105 -3634
Branches 29 19 -10
=============================================
- Hits 3530 105 -3425
+ Misses 203 0 -203
+ Partials 6 0 -6 Continue to review full report at Codecov.
|
Signed-off-by: Peter Zhu <[email protected]>
Signed-off-by: Peter Zhu <[email protected]>
Signed-off-by: Peter Zhu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR bolts the changes about specifics on OpenSearch distributions all the way down in base classes that are reusable and generic. Refactor this out into child classes and get rid of all the if
's.
@@ -12,12 +12,14 @@ | |||
|
|||
|
|||
class Process: | |||
def __init__(self) -> None: | |||
def __init__(self, filename: str, distribution: str) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Process is a generic class in system
, it should have nothing to do with distribution, this doesn't belong here.
@@ -65,6 +67,10 @@ def terminate(self) -> int: | |||
self.return_code = self.process.returncode | |||
self.process = None | |||
|
|||
if self.distribution == "rpm": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not live in Process
.
self.security_enabled = security_enabled | ||
self.additional_config = additional_config | ||
self.dependency_installer = dependency_installer | ||
|
||
self.process_handler = Process() | ||
self.process_handler = Process(self.filename, self.distribution) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class is a generic Service
. Subclass it into ServiceOpenSearchTar
and ServiceOpenSearchRpm
, ServiceOpenSearchDashboardsTar
etc., and implement the differences in the child classes.
logging.info(f"Unpacked {bundle_name}") | ||
logging.info(f"Installing {bundle_name} in {self.install_dir}") | ||
|
||
if self.distribution == "tar": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Subclass ServiceOpenSearch
into ServiceOpenSearchTar
and ``ServiceOpenSearchRpm` and implement the differences there.
Signed-off-by: Peter Zhu [email protected]
Description
Add rpm support for integTest framework
Issues Resolved
#1951
Check List
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.