-
Notifications
You must be signed in to change notification settings - Fork 271
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
Move git_repository into its own Python package and update build.py and test.py to use the new location. #186
Conversation
0d6038d
to
6714577
Compare
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.
LGTM, consider addressing some minor things below now or later
data = yaml.safe_load(file) | ||
return BundleManifest(data) | ||
|
||
@staticmethod |
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.
Is this still used?
@@ -0,0 +1,14 @@ | |||
import os |
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.
Do we want to try and be consistent between dashes and underscores in bundle-workflow
and test-workflow
?
script = self.repo.dir.name + "/integtest.sh" | ||
if (os.path.exists(script)): | ||
print(f'sh integtest.sh -b {cluster.endpoint()} -p {cluster.port()}') | ||
# repo.execute(f'sh integtest.sh -b {cluster.endpoint()} -p {cluster.port()}') |
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.
Did you mean to leave this commented out? It wasn't in the original implementation.
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.
Yeah, it's stubbed out for now until we implement the setup-and-tear-down-the-test-cluster code.
This needs a rebase. |
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.
lgtm with a rebase
6714577
to
587c7d4
Compare
Move git_repository from build_workflow into a new "git" package and update build.py and test.py to use the new location. Remove the duplicate GitRepository from test_workflow. Change test.py to use the manifests and git packages. Move bundle_manifest to the manifests package. Rename integ_test.py to integ_test_suite.py. Signed-off-by: Cameron Skinner <camerski@amazon.com>
587c7d4
to
3059b35
Compare
…ensearch-project#186) Move git_repository from build_workflow into a new "git" package and update build.py and test.py to use the new location. Remove the duplicate GitRepository from test_workflow. Change test.py to use the manifests and git packages. Move bundle_manifest to the manifests package. Rename integ_test.py to integ_test_suite.py. Signed-off-by: Cameron Skinner <camerski@amazon.com>
Move git_repository from build_workflow into a new "git" package and update build.py and test.py to use the new location.
Remove the duplicate GitRepository from test_workflow.
Change test.py to use the manifests and git packages. Move bundle_manifest to the manifests package.
Rename integ_test.py to integ_test_suite.py.
Testing
Manually verified that
build.py
andtest.py
still work.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.