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.

Sign up for GitHub

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

Jump to bottom

Issue upgrading Dagster to 1.7.15 using helm/celery/k8 worker #23268

Closed
henrytseng opened this issue Jul 26, 2024 · 4 comments
Closed

Issue upgrading Dagster to 1.7.15 using helm/celery/k8 worker #23268

henrytseng opened this issue Jul 26, 2024 · 4 comments
Labels
deployment: k8s Related to deploying Dagster to Kubernetes type: bug Something isn't working

Comments

@henrytseng
Copy link

Dagster version

dagster, version 1.7.15

What's the issue?

I'm seeing an error with after upgrading from 1.7.14 to 1.7.15.

 TypeError: create_k8s_job_task.<locals>._execute_step_k8s_job() got an unexpected keyword argument 'per_step_k8s_config'

Just a heads up, it looks like per_step_k8s_config was just added to 1.7.15 but it doesn't seem like it works with the earlier 1.7.14 image in DockerHub.

What did you expect to happen?

Minor version patch compatibility

How to reproduce?

Run celeryK8sRunLauncher with Helm chart with 1.7.14 image deployed and kick off Celery based job.

Deployment type

Dagster Helm chart

Deployment details

No response

Additional information

No response

Message from the maintainers

Impacted by this issue? Give it a 👍! We factor engagement into prioritization.
@henrytseng henrytseng added the type: bug Something isn't working label Jul 26, 2024
@gibsondan
Copy link
Member

Hi @henrytseng - just to confirm, the thing that you upgraded to 1.7.15 is the image running your code, while leaving the rest of the helm chart using the 1.7.14 release?

gibsondan added a commit that referenced this issue Jul 26, 2024
@gibsondan
back-compat improvements for celery k8s executor arguments
dd1d167 
Summary:
Resolves #23268.

#23053 added a new argument to the celery task signature and passed it in, even when it was empty. Since the celery workers might be running older versions of dagster, a new version of dagster running the executor might set this field and hit the error in that task.

Attempt to resolve that in two directions:
- only set the new argument if it is non-empty
- Add kwargs to the task signature, to hopefully make it more resilient to new arguments in the future

Test Plan: BK, run celery with user code on master but the celery workers on 1.7.14, should no longer reproduce the linked error
@gibsondan gibsondan mentioned this issue Jul 26, 2024
Closed
@alekseik1
Copy link
Contributor

Hi, @henrytseng , I'm also experiencing same issue after upgrading to 1.7.15 in package and keeping docker image 1.7.14.
I think the image with updated dagster should be used. As of right now, there is no 1.7.15 tag on celery k8s in dockerhub (looks like it was not pushed when release happened).
@gibsondan could you please build & push a 1.7.15 tag here?

@gibsondan
Copy link
Member

Thanks for flagging that - we've identified the problem that occurred there and are pushing the images now.

@garethbrickman garethbrickman added the deployment: k8s Related to deploying Dagster to Kubernetes label Jul 26, 2024
@gibsondan
Copy link
Member

The missing 1.7.15 image has now been pushed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
deployment: k8s Related to deploying Dagster to Kubernetes type: bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

back-compat improvements for celery k8s executor arguments dagster-io/dagster
4 participants
@henrytseng @gibsondan @alekseik1 @garethbrickman