readthedocs / readthedocs-docker-images

Docker image definitions used by Read the Docs
115 stars 70 forks source link

Upload new build tools #181

Closed stsewd closed 2 years ago

stsewd commented 2 years ago

Ref https://github.com/readthedocs/readthedocs.org/pull/9268

stsewd commented 2 years ago

@humitos this doesn't remove the existing ones, right?

stsewd commented 2 years ago

Reading https://docs.aws.amazon.com/cli/latest/reference/s3/sync.html you need to pass --delete for that to happen, so looks like it won't delete existing files.

agjohnson commented 2 years ago

Should we wait to merge this on release day? Or I suppose line it up so everything is ready as we start release?

humitos commented 2 years ago

@stsewd

@humitos this doesn't remove the existing ones, right?

No, it will override the existing ones with the new ones just built.

@agjohnson

Should we wait to merge this on release day? Or I suppose line it up so everything is ready as we start release?

I think merging this PR during release is the best, so we are all watching this in case we need to take some action.

humitos commented 2 years ago

BTW, we need to make the tests to pass to be able to merge this PR because otherwise, it won't upload anything to S3 😄

humitos commented 2 years ago

@humitos this doesn't remove the existing ones, right?

No, it will override the existing ones with the new ones just built.

@stsewd is right here. We are adding the version in the filename. I didn't remember that. So, in case we need to revert, we just revert the application PR and that's it. This case was considered as part of the design and I didn't remember it 😅

That said, I'm happy to merge it.

agjohnson commented 2 years ago

What are the test failures? Looks like it failed to grab our 20.04 image for some reason?

stsewd commented 2 years ago

Tests seem to have been failing since March

agjohnson commented 2 years ago

Ah nevermind, I see what's happening. It's building the 22.04 image in tests, so that's why it can't find the 20.04 image

agjohnson commented 2 years ago

Thanks @stsewd! Merging :ship: