Skip to content

Display priority values as specified by user (removing the negative sign)#4211

Open
SultanOrazbayev wants to merge 3 commits into
dask:mainfrom
SultanOrazbayev:patch-2
Open

Display priority values as specified by user (removing the negative sign)#4211
SultanOrazbayev wants to merge 3 commits into
dask:mainfrom
SultanOrazbayev:patch-2

Conversation

@SultanOrazbayev

Copy link
Copy Markdown
Contributor

Only the first element in the tuple is negated (it contains the user-specified priority), other components are unchanged.

This is a follow-up from #4185

@jrbourbeau jrbourbeau left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR @SultanOrazbayev. Apologies for the CI build failures, they are unrelated to the changes in this PR. I've merged the main branch into this branch which should fix the test failures (hope that's okay)

@mrocklin

mrocklin commented Nov 3, 2020

Copy link
Copy Markdown
Member

Only the first element in the tuple is negated (it contains the user-specified priority), other components are unchanged.

Hrm, that's interesting. We probably can't negate only one element, otherwise the presented value doesn't actually represent the priority. We probably have to negate them all or none.

@SultanOrazbayev

Copy link
Copy Markdown
Contributor Author

Uh oh. I thought just the first element should be negated because of these lines:

for key in set(priority) & touched_keys:
ts = self.tasks[key]
if ts.priority is None:
ts.priority = (-(user_priority.get(key, 0)), generation, priority[key])

In the dashboard right now (without this commit), the negative sign appears only on the user-specified values... so I thought this would fix it.

@mrocklin

mrocklin commented Nov 3, 2020 via email

Copy link
Copy Markdown
Member

Base automatically changed from master to main March 8, 2021 19:04
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