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

Patch long lived DaskCluster components on update #936

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

edgar-s-silva-alb
Copy link

Implementation for #895.

I'm not super familiar with kopf and kubernetes code in general, so some input would be appreciated.

@edgar-s-silva-alb
Copy link
Author

@jacobtomlinson Hey, from my testing this solution is not currently working fully, but does work on some cases with the scheduler.
I would like to get on your input if this looks like the right path.

@edgar-s-silva-alb edgar-s-silva-alb force-pushed the operator-cluster-update branch from 2824d76 to 8a51e23 Compare April 2, 2025 11:07
Copy link
Member

@jacobtomlinson jacobtomlinson left a comment

Choose a reason for hiding this comment

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

This approach generally looks reasonable to me. Feel free to ping me for full review when ready.

Could you make sure you're running pre-commit to fix up linting issues. It looks like your editor has changed a bunch of things that are formatting related.

Comment on lines +392 to +393
assert name
assert namespace
Copy link
Member

Choose a reason for hiding this comment

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

If either of these assertions fail then kopf will retry automatically forever. We probably need to catch the assertion error and just return if this isn't what we want to hapen.

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