Skip to content

fixed _cleanup_pods auth using kubectl config - #172

Closed
giordyb wants to merge 1 commit into
dask:masterfrom
giordyb:master
Closed

fixed _cleanup_pods auth using kubectl config#172
giordyb wants to merge 1 commit into
dask:masterfrom
giordyb:master

Conversation

@giordyb

@giordyb giordyb commented Aug 15, 2019

Copy link
Copy Markdown

Hi, as requested here is the pull request to fix issue #170.

Since _cleanup_pods is only called from the Cluster object I thought it would be cleaner to just avoid creating a new kubernetes.client.CoreV1Api() object and just re-use the one created on _init by passing it as a argument.

thanks,

Giordano

@jakirkham

Copy link
Copy Markdown
Member

I think Jacob's out today, but should be back in a day. Hopefully he can take a look then 🙂

@jacobtomlinson

Copy link
Copy Markdown
Member

Thanks for this! These CI issues should be fixed after #171. Holding until then.

@jacobtomlinson

Copy link
Copy Markdown
Member

Since #162 has been merged this now has conflicts.

Also one thing we discussed as part of that PR was adding a change like this. Sadly due to switching to asyncio for the kubernetes API we can't guarantee that the loop is still running at finalize time. This means passing the now async API object to the finalize call isn't suitable. In the rewrite, we actually create a new sync version of the API object during cleanup.

Thanks anyway for making this change! If you have thoughts or comments we would be keen to hear them.

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.

3 participants