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

[dagster-k8s] per_step_k8s_config working with dynamic jobs #26670

Merged
merged 2 commits into from
Dec 26, 2024

Conversation

abhinavDhulipala
Copy link
Contributor

@abhinavDhulipala abhinavDhulipala commented Dec 22, 2024

Summary & Motivation

Allow run time configuration of dynamic steps using the k8s_job_executor. Previously, dynamic steps would not be configured when using per_step_k8s_config. Now doing something along line of the following:

execution:
  config:
    per_step_k8s_config:
      dyn_sink:
         container_config:
           resources:
             requests:
               cpu: '300m'

Given the following job:

@job
def dynamic_producer_consumer_job():
    @op(out=DynamicOut(int))
    def dyn_producer():
        yield from (DynamicOutput(i, str(i)) for i in range(3))
    @op
    def dyn_sink(producer: int): ...

Will configure all dyn_sink ops with the following modified config.

cc @Kuhlwein

resolves #26588

How I Tested These Changes

Red green coverage given a dynamic job. Revert the following line to watch the test fail.

CHANGELOG

  • [dagster-k8s] k8s_job_executor supports per step configuration for dynamic steps.

@abhinavDhulipala abhinavDhulipala changed the title [dagster-k8s] per_step_config working with dynamic jobs [dagster-k8s] per_step_k8s_config working with dynamic jobs Dec 22, 2024
@mlarose mlarose requested a review from gibsondan December 26, 2024 18:20
@mlarose
Copy link
Contributor

mlarose commented Dec 26, 2024

Hi @abhinavDhulipala! thanks for the contribution.

I think you have a small typo in your description. Could you please fix the example to use per_step_k8s_config, for posterity?

Otherwise lgtm, adding a colleague to double check on the switch from step_key to op_name

@abhinavDhulipala
Copy link
Contributor Author

Thanks for the catch @mlarose . Description fixed 👍

@mlarose mlarose merged commit bc84a5a into dagster-io:master Dec 26, 2024
1 check passed
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.

[dagster-k8s] per op config fails on dynamic op graph
3 participants