Page MenuHomeElementl

Include container_image in recon_pipeline_from_origin and recon_repository_from_origin,
ClosedPublic

Authored by dgibson on May 13 2021, 4:48 AM.

Details

Summary

This was presumably an oversight - these origins have container images, they should be included in the resulting ReconstructableRepository/ReconstructablePipeline.

The result of this is that we can use the origin in various executors rather than relying on DAGSTER_CURRENT_IMAGE being set in the run launcher.

Test Plan

Integration

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dgibson edited the test plan for this revision. (Show Details)
dgibson retitled this revision from Include container_image in recon_pipeline_from_origin and recon_repository_from_origin to Include container_image in recon_pipeline_from_origin and recon_repository_from_origin,.
dgibson edited the summary of this revision. (Show Details)

up

This LGTM

python_modules/libraries/dagster-celery-k8s/dagster_celery_k8s/executor.py
232

Current practice in the launchers is to error here instead of overriding https://github.com/dagster-io/dagster/blob/master/python_modules/libraries/dagster-celery-k8s/dagster_celery_k8s/launcher.py#L167-L176

I don't know that there's a strong reason for it though

This revision is now accepted and ready to land.Mon, May 17, 10:18 PM