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

Remove duplication of GraphLayout scheduler plugin #5294

Open
douglasdavis opened this issue Sep 7, 2021 · 4 comments · May be fixed by #5318
Open

Remove duplication of GraphLayout scheduler plugin #5294

douglasdavis opened this issue Sep 7, 2021 · 4 comments · May be fixed by #5318

Comments

@douglasdavis
Copy link
Member

Following up on a discovery in #5267: the GraphLayout plugin is duplicated in the scheduler plugins. The latest release uses uuid to generate a unique name for each instance to avoid the duplication warning; we should be able to remove this work around.

@jrbourbeau
Copy link
Member

Thanks for tracking this @douglasdavis. Just checking in, do you have the bandwidth to look into this? No worries if not

@douglasdavis
Copy link
Member Author

douglasdavis commented Sep 8, 2021 via email

@douglasdavis
Copy link
Member Author

Still digging but jotting down what I've tracked down so far.

TaskGraph.__init__ creates a GraphLayout instance and adds it to the scheduler plugins.

In distributed/dashboard/scheduler.py, the applications dictionary has two places where a TaskGraph is created:


"/individual-graph": individual_doc(TaskGraph, 200),

graph_doc creates a TaskGraph instance and individual_doc creates an instance of the class passed as the first argument.

test_simple loops over that dictionary triggering the creation of two TaskGraph instances:

for suffix in applications:
response = await http_client.fetch(f"http://localhost:{port}{suffix}")
body = response.body.decode()
assert "bokeh" in body.lower()
assert not re.search("href=./", body) # no absolute links

What you tried in #5267, to only create a GraphLayout plugin in TaskGraph.__init__ if scheduler.plugins didn't already contain a plugin with name graph-layout, is the first solution I thought of. But it looks like that wasn't a good fix. Will keep trying some alternatives.

@douglasdavis
Copy link
Member Author

douglasdavis commented Sep 10, 2021

Since these are two different dashboard endpoints would it make sense to have two GraphLayout plugin instances? In that case we can just pass a kwarg to TaskGraph.__init__ to give its GraphLayout a name. @jrbourbeau what do you think?

@douglasdavis douglasdavis linked a pull request Sep 14, 2021 that will close this issue
3 tasks
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 a pull request may close this issue.

2 participants