Skip to content

Conversation

@douglasdavis
Copy link
Member

@douglasdavis douglasdavis commented Aug 24, 2021

  • Closes #xxxx
  • Tests added / passed
  • Passes black distributed / flake8 distributed / isort distributed

cc @jrbourbeau; this locally fixes what popped up in #5257 (comment). Definitely open to discussion on a different style of fix. I'm looking to see how that test causes an update of the dictionary during iteration

Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks @douglasdavis! Looking at the diff for #5120 there were several places where we were previously using list(...) or taking a slice for plugin in self.plugins[:] (search for for plugin in when looking at the diff), would you mind making updates in those other places too?

@douglasdavis
Copy link
Member Author

douglasdavis commented Aug 24, 2021

Sure -- went through and adjusted all of the other instances of self.plugins.values() to list(self.plugins.values()). Looks like one worker plugin associated test and an async test are failing. Not sure if they're related to this. (they pass locally for me)

Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks @douglasdavis! I'm also not able to reproduce that failure locally. Let's just keep an eye out for it

@jrbourbeau jrbourbeau merged commit dc47186 into dask:main Aug 24, 2021
@douglasdavis douglasdavis deleted the sched-plugin-iteration branch August 24, 2021 19:57
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