i think building images to make dagster core changes is not the most optimal dev loop?

sending back to your queue w/ a few things

• I think that job_namespace, load_incluster_config, kubeconfig_file for the K8sRunLauncher should be configurable from the helm chart, similar to other system components
• re "It's kinda weird to have the launcher rely on the pipeline run configmap on when it might not even be configured if the user decides to go with user deployments." -- when a user enables user deployments, the pipeline run confipmap can still be used by the K8sScheduler. I agree that the naming is a bit dated, but I would much prefer either using the env-pipeline-run configmap, renaming the env-pipeline-run configmap, or deprecating the env-pipeline-run configmap (probably in a separate diff?) instead of having two configmaps that are very similar.
• wrt documentation, i generally prefer adding docs the same time a feature is released so that users can immediately have a place to learn more; its also a good time to reflect on the code before committing to see if anything could be improved, and guarantees that the documentation actually gets written instead of backlogged

awesome, looks good!

looking good! few things (1) whats the reasoning behind adding "helm/dagster/templates/configmap-env-instance.yaml" instead of using "helm/dagster/templates/configmap-env-pipeline-run.yaml"? (2) in test_integration.py, could we assert that there are no celery worker pods and no step job pods? in a previous migration, there was a bad issue where the tests were no longer using the expected run launcher so would be nice to add a sanity check

up

@alangenfeld re: telemetry, @rexledesma added telemetry for launch_scheduled_execution and is creating a doc to share (1) how to add telemetry to other functions and (2) how to query the collected data via sql. Are you thinking of adding telemetry for workspace_from_load_target, including the type of load_target -- should be similar to log_repo_stats unless we want to intercept the load_target within the telemetry_wrapper

Since the extraWorkerQueues part of this test has caused flakiness in the past, can we make sure to run this through bk a few times (5?) times before merging? last time i tried to add extraWorkerQueues back, it passed the first three runs and then failed twice (https://dagster.phacility.com/D4535)

looks reasonable to me! can we add a test that involves spinning up a k8s-only deployment in kind and asserting that we can run a pipeline successfully?

approving conditional on checking for engine events (ie "Sending run coordinator task to Celery" + "Run coordinator finished, Celery exiting") in tests and some error handling for if "wait_for_job_success" raises an exception

add test?

u

remove extra files

we might want to add an engine event saying that there was a user-initiated termination

update DefaultRunLauncher terminate() to only call either cli api run launcher's terminate() or grpc run launcher's terminate() but not both

bk flake

update test

use error=serializable_error_info_from_exc_info(sys.exc_info()),

up

add cls to engine event

isort

up

rebase

rebase

rebase

I think there might be a few other occurrences, like https://github.com/dagster-io/dagster/blob/master/python_modules/libraries/dagster-k8s/dagster_k8s/client.py#L200 and making it more explicit in https://github.com/dagster-io/dagster/blob/master/python_modules/libraries/dagster-celery-k8s/dagster_celery_k8s/executor.py#L310

nice catch

update comment

run bk again

sooooo awesome