Break a generic copy function out of deploy_to_s3. #19251
Conversation
|
I've verified that deploy_to_s3.py and backfill_s3_release_tag_mappings.py generate identical aws commands before and after the change. |
|
It seems like it would be more prudent to use https://docs.github.com/en/actions/using-workflows/storing-workflow-data-as-artifacts and save money. Was that considered? |
| ) | ||
| except subprocess.CalledProcessError as ex: | ||
| logger.error(f"Process `{' '.join(args)}` failed with exit code {ex.returncode}.") | ||
| logger.error(f"stdout:\n{ex.stdout.decode()}\n") |
There was a problem hiding this comment.
You could text=True the run call and save a bit of hoops here.
There was a problem hiding this comment.
I just copied that from its previous location unchanged.
I thought about it, and we could. Downloading those is a bit annoying because you have to know the RUN_ID of the run, unless you download the artifact for all runs. So there would be more GH API tennis. The S3 cost of storing the XML files should be completely in the noise compared to our current heavyweight use of S3 for binaries, and our new use of it for remote caching. If we decide we want to move off S3 (that's probably sensible to consider) I would try and go by order of size/cost. |
This will allow us to support copying non deploy-related
files to s3 during CI.
The first example (in a follow-on change) being the pytest
XML output files, so we can gather test performance data.