Fix issues with translation deployment script #10667
Conversation
During the 0.25.1/0.44.1 release the translation publishing step failed. This was because of a parsing error with the ssh private key during the decryption step. This seems to be due to how the action was configured to load the private key post decryption into an env variable that github actions would then use for it's checkout action to handle cloning with authentication via ssh. This commit switches back to just using a standalone bash script that has proven to be reliable historically and also gives us tighter control on how commands get executed. As part of this a new encrypted private key is added to the repo as the previous key has been revoked and invalidated since the job failure during the release.
mtreinish
added
type: qa
|
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 5903498041
💛 - Coveralls |
There was a problem hiding this comment.
It was pretty poor work on my part not to have better tested the secrets management in the new form - I'd removed that section so I could test the rest of the script, but I should have used dummy credentials. I think I just ran out of time on the way out to holiday, but I really should have thought of it.
I have a vague preference towards the new form (but I can fix it so it doesn't leak the secret lol), since I personally find it much easier to read and it uses more of the GitHub Actions machinery to efficiently manage git operations and secret management, but I'm not super broken up about it. Originally I'd intended to completely reuse much more of the Python environments, but tox doesn't make that especially easy, so it's probably not so big a deal until we have something like a lockfile for the artifact generation for docs.
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Co-authored-by: Jake Lishman <jake@binhbar.com>
There was a problem hiding this comment.
I'm trying to remember all the changes to this script that I made when I moved it over to fix problems with it, but it's a bit bits-and-pieces because I did it a couple of weeks ago now in the midst of a lot of other busy stuff, and I don't remember everything.
Would you prefer if I try and fix the secret sharing on the new form of the action, or would you much prefer to revert to the stand-alone script?
| @@ -80,7 +80,7 @@ git add setup.py | |||
| git add requirements-dev.txt constraints.txt | |||
There was a problem hiding this comment.
This line and the setup above it is wrong now as well - we need to clone more stuff into the translations repository if they're going to be able to build the Terra documentation like this.
There was a problem hiding this comment.
This line should actually be fine because of L74. The requirements-dev.txt should include all the dependencies needed to build the docs. The translations CI will install a bit more than they were before but I don't think that's a huge issue. But yeah the setup.py will be a problem, it should be qiskit_pkg/setup.py
There was a problem hiding this comment.
Ah, sorry, yeah - so much of the file was completely identical that I'd missed that you'd included a couple of things above. Still, yeah, I think the current setup.py would break.
There was a problem hiding this comment.
Actually I just went to update the setup.py and it's correct too, at L70 I have it copy the setup.py from $SOURCE_DIR/qiskit_pkg/setup.py to the root of the translations checkout, so adding the setup.py here is adding the expected file.
There was a problem hiding this comment.
I'm not a huge fan of how much we're mixing-and-matching bits and bobs of configuration files here to make a semi-functional package for the translations repo, but I don't have any better solution, so I guess let's just move to merge and get these rolling again.
Co-authored-by: Jake Lishman <jake@binhbar.com>
| @@ -80,7 +80,7 @@ git add setup.py | |||
| git add requirements-dev.txt constraints.txt | |||
There was a problem hiding this comment.
I'm not a huge fan of how much we're mixing-and-matching bits and bobs of configuration files here to make a semi-functional package for the translations repo, but I don't have any better solution, so I guess let's just move to merge and get these rolling again.
* Fix issues with translation deployment script During the 0.25.1/0.44.1 release the translation publishing step failed. This was because of a parsing error with the ssh private key during the decryption step. This seems to be due to how the action was configured to load the private key post decryption into an env variable that github actions would then use for it's checkout action to handle cloning with authentication via ssh. This commit switches back to just using a standalone bash script that has proven to be reliable historically and also gives us tighter control on how commands get executed. As part of this a new encrypted private key is added to the repo as the previous key has been revoked and invalidated since the job failure during the release. * Apply suggestions from code review Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Co-authored-by: Jake Lishman <jake@binhbar.com> * Update tools/deploy_translatable_strings.sh Co-authored-by: Jake Lishman <jake@binhbar.com> --------- Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Co-authored-by: Jake Lishman <jake@binhbar.com> (cherry picked from commit cd1196c)
* Fix issues with translation deployment script During the 0.25.1/0.44.1 release the translation publishing step failed. This was because of a parsing error with the ssh private key during the decryption step. This seems to be due to how the action was configured to load the private key post decryption into an env variable that github actions would then use for it's checkout action to handle cloning with authentication via ssh. This commit switches back to just using a standalone bash script that has proven to be reliable historically and also gives us tighter control on how commands get executed. As part of this a new encrypted private key is added to the repo as the previous key has been revoked and invalidated since the job failure during the release. * Apply suggestions from code review Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Co-authored-by: Jake Lishman <jake@binhbar.com> * Update tools/deploy_translatable_strings.sh Co-authored-by: Jake Lishman <jake@binhbar.com> --------- Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Co-authored-by: Jake Lishman <jake@binhbar.com> (cherry picked from commit cd1196c) Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Summary
During the 0.25.1/0.44.1 release the translation publishing step failed. This was because of a parsing error with the ssh private key during the decryption step. This seems to be due to how the action was configured to load the private key post decryption into an env variable that github actions would then use for it's checkout action to handle cloning with authentication via ssh. This commit switches back to just using a standalone bash script that has proven to be reliable historically and also gives us tighter control on how commands get executed. As part of this a new encrypted private key is added to the repo as the previous key has been revoked and invalidated since the job failure during the release.
Details and comments