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

Show number of unread worker messages in UI #2013

Merged
merged 14 commits into from
Feb 6, 2017

Conversation

riga
Copy link
Contributor

@riga riga commented Feb 3, 2017

This PR adds a feature as proposed in #1993.

In the UI, the number of unread messages per worker is shown near the top right tool box.

Changes are quite trivial and should require any additional tests =)

@erikbern
Copy link
Contributor

erikbern commented Feb 3, 2017

hm 13 commits for 10 lines of code? :)

Copy link
Contributor

@Tarrasch Tarrasch left a comment

Choose a reason for hiding this comment

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

Yea. This is what I wanted. Although tests wouldn't have hurt. :)

@@ -1328,6 +1328,7 @@ def worker_list(self, include_running=True, **kwargs):
started=worker.started,
state=worker.state,
first_task_display_name=self._first_task_display_name(worker),
unread_messages=len(worker.rpc_messages),
Copy link
Contributor

Choose a reason for hiding this comment

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

Call this num_unread_messages? Or even better, num_unread_rpc_messages. Would be nice if a grep-search for rpc_messages turned out all relevant bits of code for the RPC messages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, sounds good. Done.

#workerList .box-tools > span.label-unread-worker-messages {
margin-right: 6px;
font-style: italic;
vertical-align: middle;
Copy link
Contributor

Choose a reason for hiding this comment

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

What color do you have it in? I was thinking of maybe making it red, as it's something that under normal conditions quickly disappears.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also done ;)

@riga
Copy link
Contributor Author

riga commented Feb 4, 2017

@erikbern Sorry for that. I merged with some changes from @Tarrasch on the master (d0dcac6). Should have opened a new branch...

@Tarrasch Tarrasch merged commit d5d9f58 into spotify:master Feb 6, 2017
@Tarrasch
Copy link
Contributor

Tarrasch commented Feb 6, 2017

@erikbern @riga, thankfully GitHub now has squashmerge, so we can merge this without fixing the strange-looking history in this PR.

Awesome job @riga, thanks for fast fix. :)

kreczko pushed a commit to kreczko/luigi that referenced this pull request Mar 28, 2017
This was referenced Jun 29, 2022
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