WIP [ART-7190] Switch to openshift-eng/art-tools #3860
Conversation
ashwindasr
added
the
do-not-merge/hold
|
Do not merge before @ashwindasr runs the code to sync doozer and elliott to art-tools |
|
Can you include a test job? |
| @@ -164,7 +164,9 @@ def setup_venv(use_python38=false) { | |||
| commonlib.shell(script: "rm -rf art-tools/elliott ; cd art-tools; git clone https://github.com/${where[0]}/elliott.git; cd elliott; git checkout ${where[1]}") | |||
There was a problem hiding this comment.
doozer_commit and elliott_commit params would stop working, and I think we want to have a art_tools_commit / tools_commit in place of them (see promote / prepare release / build-sync accepting them)
There was a problem hiding this comment.
I see, let me see if I can understand whats happening. But when we ran the prepare release test job as dry run, it worked.
There was a problem hiding this comment.
For prepare-release the change was recent and the job run is older (13 days ago) so it doesn't have the elliott_commit param. To see it break you would have to run it with a elliott_commit value like ashwindasr@some_branch_name
See #3847
There was a problem hiding this comment.
Understood, will start working on those as well and will add to this PR
| @@ -164,7 +164,9 @@ def setup_venv(use_python38=false) { | |||
| commonlib.shell(script: "rm -rf art-tools/elliott ; cd art-tools; git clone https://github.com/${where[0]}/elliott.git; cd elliott; git checkout ${where[1]}") | |||
| } | |||
|
|
|||
| commonlib.shell(script: "pip install -e art-tools/elliott/ -e art-tools/doozer/ -e pyartcd/") | |||
| commonlib.shell(script: "cd art-tools; rm -rf art-tools; git clone https://github.com/openshift-eng/art-tools.git; cd art-tools; git checkout master") | |||
There was a problem hiding this comment.
How does this work? Are we not having art-tools as a submodule to aos-cd-jobs (sorry if i forgot a previous discussion) ?
There was a problem hiding this comment.
Well, I wasn't sure about the stance about sub-modules. The long term vision was to replace the art-tools directory with the art-tools repo on the root level of aos-cd-jobs. Not sure if we wanted to clone or do submodule. But for the purposes of this PR, we need art-tools in the same directory as doozer and elliott submodules, so if you think its a good idea, I can add that as a submodule instead of cloning.
There was a problem hiding this comment.
I like our current pattern of keeping it as a submodule, and let's keep it on the root level so it's not art-tools/art-tools?
There was a problem hiding this comment.
Ahh regarding that, I wanted to keep this PR reversible. If something happens and we need to roll back, we just need to change the paths to the old doozer and elliott submodules. I was thinking if after a couple of releases we don't have any issues, I'll swap in the whole art-tools folder in the root level. What do you think?
There was a problem hiding this comment.
What will go wrong though? :)
If we've tested things and it works I don't see us rolling back. And if in case we do need to rollback, we'll revert this PR, and add back individual submodules - does not sound much effort to me. I would say let's go forward and not think of going back, we fix issues as they come up
There was a problem hiding this comment.
Makes sense. I was trying to follow a defensive approach, but with the brainpower of our team, I think we can push forward
Apologies, have added now |
thegreyd
changed the title
|
/retitle [ART-7190 Switch to openshift-eng/art-tools |
openshift-ci
bot
changed the title
|
/assign @ashwindasr |
|
/retitle [ART-7190] Switch to openshift-eng/art-tools |
openshift-ci
bot
changed the title
|
/unhold /wip |
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/wip |
|
/retitle WIP [ART-7190] Switch to openshift-eng/art-tools |
openshift-ci
bot
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Closing in favor of #3881 |
Switch to
openshift-eng/art-tools. Ref ART-7190Gen-assembly test run.
Prepare-release test run