no HBO max issues for us

Ah, termination is in the next diff

unrelated test failure

overall this makes sense to me - tweak some names and remove some extra timezone fields and i think we're good

missed a spot

up

My understanding is we're not skipping step 6, but it's not doing exactly what you think it is. "dagster-release release" doesn't increment the dagster version in the virtual environment (it doesn't modify it in place, it does a git push to the repository in a separate checkout). So it does incrementsthe dagster version if you do a fresh checkout of the release branch. If we re-generated the virtual environment, it would now be on the correct version, which is why the pipeline succeeds if you re-run it and rebuild the virtual environment.

probably will require a quick internal change too right?

er, my mistake, it's running using the build environment (which is on the release branch but doesn't have the newly written dagster version that the dagster-release release command created - the code is correct, the version is not. That's why it works when you re-run the pipeline)

tweaks

add @owen's suggestion and this looks good to me

An option that lets you pass in a custom rev does seem like a good idea too (in the case before, we could have set them to the pre-0.10.0 rev that we expected them to be at)

It seemed to work with the specific migration that prompted this (0.10.9 => 0.11.13)

landing this now so that there aren't two dagster-release CLIs during the release (one in automation and one in automation-internal) - will revert if the internal release plan today doesn't work out for some reasn

sweet, thank you for powering through that, the universe makes sense again :)

yeah to johann's point - the docker-compose setup here is basically identical other than that the image comes from $REGISTRY_URL right? (and the RunLauncher will be an ECSRunLauncher eventually - although will that similarly be able to use the DockerRunLauncher with different Docker creds do you think?)

want to grab the one in celery_k8s_job_executor too? I think it's similarly not needed

Makes sense to me - there's an internal version coming too i assume

I like that idea

thanks for doing this! One small thing and we're good I think (let's merge create() with the constructor)

seems reasonable to me then

leaving open for a bit for the question for jordan

sorry for the delay, this one took me a while to wrap my brain around.