Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

K8sJobExecutor: Respect executor settings, specified on job launch #26941

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

msavtchouk-pf
Copy link

Summary & Motivation

Allow k8s_job_executor configuration on job launch, including step_k8s_config and per_step_k8s_config.
Previously, when run_configuration was specified on the job launch, i.e. using grapql interface, this configuration was completely ignored. See example below.

client = dagster_graphql.DagsterGraphQLClient()

run_config = dag.RunConfig(
    execution={
        "config": {
            "step_k8s_config": {"container_config": {"resources": {"requests": {"cpu": "100m"}}}},
            "per_step_k8s_config": {
                "dyn_sink": {"container_config": {"resources": {"requests": {"cpu": "100m"}}}},
            },
        }
    },
)
client.submit_job_execution(
        dagster_job_name, run_config=run_config)

How I Tested These Changes

I have implemented a test that tests this change. In addition, tested with a patched custom executor on local installation of k8s dagster

Changelog

Insert changelog entry or delete this section.

Copy link
Member

@gibsondan gibsondan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @msavtchouk-pf - thank you for sending this out and I agree the underlying problem here is annoying that you need to set run and step config differently, but this isn't something that we're currently planning to change. executor config currently only affects the executor rather than the run launcher, and its not uncommon for steps and runs to have different configuration (especially resources).

Typically the inheritance works in the opposite direction - configuration on runs is applied down to steps if you're using the k8srunlauncher, but can be overridden at the step level using executor config.

I could imagine us adding some kind of 'run launcher config' to run config akin to executor config that allows you to configure the run launch params as well via the launchpad, but that would be a larger change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants