-
Notifications
You must be signed in to change notification settings - Fork 706
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
Use up to date build_ds_container
script in add_platform_rule.py
#11042
Use up to date build_ds_container
script in add_platform_rule.py
#11042
Conversation
utils/add_platform_rule.py
Outdated
@@ -277,7 +277,7 @@ def clusterTestFunc(args): | |||
print('* Pushing image build to cluster') | |||
# execute the build_ds_container script | |||
buildp = subprocess.run( | |||
['utils/build_ds_container.sh', '-P', 'ocp4', '-P', 'rhcos4']) | |||
['utils/build_ds_container.py']) |
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.
I think the python version still supports passing in different products. Should we maintain that?
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, I think we should keep support for different products in build_ds_container.py
.
Switch to the python script. And fix the product arguments, passing '-P' twice was causing it to be reset.
c40d002
to
8a95232
Compare
@@ -277,7 +277,7 @@ def clusterTestFunc(args): | |||
print('* Pushing image build to cluster') | |||
# execute the build_ds_container script | |||
buildp = subprocess.run( | |||
['utils/build_ds_container.sh', '-P', 'ocp4', '-P', 'rhcos4']) | |||
['utils/build_ds_container.py', '-P', 'ocp4', 'rhcos4']) |
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.
I added back explicit specification of the products to build.
But I wonder if we should just build whatever is default in build_ds_container.py
.
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.
I believe this script is OpenShift-specific. I'm OK only building ocp4
and rhcos4
products for now.
Code Climate has analyzed commit 8a95232 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 53.8% (0.0% change). View more on Code Climate. |
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
Description:
Having the '-P' twice caused the script to build only one of the products, the one defined later.
Rationale:
utils/build_ds_container.sh
is deprecated.