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

flux-jobs: support emoji output formats #4687

Merged
merged 4 commits into from
Oct 25, 2022

Conversation

chu11
Copy link
Member

@chu11 chu11 commented Oct 13, 2022

Problem: flux lacks a certain cuteness factor. This limits its potential to be liked by younger HPC users.

Solution: Support the {state_emoji}, {result_emoji}, and {status_emoji} fields, which will output the emojis representing job states
or results respectively.

Fixes #4681

image

Note, while this was originally brought up as a joke in a discussion, this isn't actually that un-useful. When listing a bunch of jobs, it may be easier to visually scan for a poop instead of printed text like FAIL. So I selected emojis based on some visual "scan-ability".

Emoji alignment is difficult, but largely ignored it as it's going to be font and terminal specific.

Let the bike-shedding of emoji choices begin!

@vsoch
Copy link
Member

vsoch commented Oct 13, 2022

Take my money now!! 😍

@garlick
Copy link
Member

garlick commented Oct 14, 2022

Hotline, get ready.

User: Flux gave my job a poop emoji and I don't know why

Hotline: Hmm, your job was canceled so it should be a plucky little bomb. What sort of poop is it?

User: it has a maniacal grin

Hotline: I'm not familiar with that job result. Are you sure it wasn't exploding in some way?

User: not really

Hotline: (consults Flux documentation) Could it have been "Shocked Face with Exploding Head"? How about "Potato"?

@ELCapitanLLNL
Copy link

I am glad you are not using the thumbs-up emoji. On todays news: Thumbs-up emoji may hurt Gen Z! https://www.foxnews.com/lifestyle/gen-z-says-emoji-passive-aggressive-you-using-it

@chu11
Copy link
Member Author

chu11 commented Oct 14, 2022

Sadly, it is difficult to capture jobs stuck in the CLEAN state: https://unicode-table.com/en/1F9FB/

User: my job was stuck in the toilet paper state, then later flux said it was poop

@ELCapitanLLNL
Copy link

ELCapitanLLNL commented Oct 14, 2022

Sadly, it is difficult to capture jobs stuck in the CLEAN state: https://unicode-table.com/en/1F9FB/

User: my job was stuck in the toilet paper state, then later flux said it was poop

Hotline: Please flux it down...

@trws
Copy link
Member

trws commented Oct 14, 2022

Sadly, it is difficult to capture jobs stuck in the CLEAN state: https://unicode-table.com/en/1F9FB/

User: my job was stuck in the toilet paper state, then later flux said it was poop

🧼 or ✨ or perhaps even ❇️ might work for clean. I suppose 🧹 could too.

@vsoch
Copy link
Member

vsoch commented Oct 14, 2022

I like the ✨ … Mr Clean approved!

@chu11
Copy link
Member Author

chu11 commented Oct 14, 2022

Oops, I really mean the "CLEANUP" state, so I think 🧼 or 🧹 seem like pretty good options. I considered 🗑 as well, but felt that was where trash goes, not the act of cleaning up.

These are critically important decisions we're making here. Gonna make sure we get this right!

@vsoch
Copy link
Member

vsoch commented Oct 14, 2022

🫧 or 🧽?

@trws
Copy link
Member

trws commented Oct 14, 2022 via email

@chu11
Copy link
Member Author

chu11 commented Oct 14, 2022

ok, we'll go with broom ... mainly b/c the sponge looks like a peanut in some other font sets.

any opinions on

depend - ✋
sched - 📅
priority - 🏎

i think sched is pretty good (i tried clock but didn't like it as much), but depend and priority are meh ...

@lucpeterson
Copy link

What about 🚦for priority?

@trws
Copy link
Member

trws commented Oct 14, 2022 via email

@vsoch
Copy link
Member

vsoch commented Oct 14, 2022

Oh I really like the orange - it stands out nicely.

@chu11
Copy link
Member Author

chu11 commented Oct 14, 2022

What about vertical_traffic_lightfor priority?

Oh I like that for priority, i.e. like "waiting for green" / "waiting to go" .... although I guess that could be for depend too.

@chu11
Copy link
Member Author

chu11 commented Oct 14, 2022

@trws ohh pause is a good idea, i like that one for depend.

@chu11
Copy link
Member Author

chu11 commented Oct 14, 2022

doh! the pause button displayed poorly in gnome (just two vertical bars). And for some reason, gnome is missing a chunk of emojis at the higher numbers, dunno if it was a more recent unicode addition and gnome just doesn't have in my version (which would be used by a fair amount of staff at the lab). So broom and soap didn't display.

So I went with

new -> wrapped gift (note, should be virtually impossible to ever see)
depend -> stop sign (per @trws suggestion above)
priority - vertical traffic light (per @lucpeterson suggestion above)
cleanup - wastebasket
inactive - skull

image

I've been using this dinky script to test if anyone wants to try on their terminal. flux start ./emoji.sh

(you may need to adjust the number of tasks to get both running & pending jobs on your machine)

#!/bin/sh
flux mini run /bin/true
flux mini run /bin/false
flux mini run -t=1s sleep 10
id=`flux mini submit sleep 30`
sleep 1
flux job cancel $id
flux mini submit -n 12 sleep 60
flux mini submit -n 12 sleep 60
flux mini submit -n 12 sleep 60
flux mini submit -n 12 sleep 60

flux jobs -a --format="{id:>12} {status:<10} {state_emoji:<5} {result_emoji:<6} {status_emoji:<6} {nodelist}"

@grondo
Copy link
Contributor

grondo commented Oct 14, 2022

(you may need to adjust the number of tasks to get both running & pending jobs on your machine)

flux jobs --from-stdin can take a list of entries on stdin so you don't have to jump through hoops to get a set of jobs in desired states or fields. This kind of testing is exactly why that option was added.

Also, I'll just mention that flux mini submit --cc=1-4 -n 12 sleep 60 is 4x faster than the serialized calls to flux mini submit in the example script.

@chu11
Copy link
Member Author

chu11 commented Oct 14, 2022

flux jobs --from-stdin can take a list of entries on stdin so you don't have to jump through hoops to get a set of jobs in desired states or fields. This kind of testing is exactly why that option was added.

Oh yeah, i totally forgot!

@chu11
Copy link
Member Author

chu11 commented Oct 15, 2022

For the life of me I cannot get the wastebasket to align, i think its width is different than the rest:

image

#!/bin/sh
flux jobs -a --format="{id:>12} {status:<10} {state_emoji:<6} {result_emoji:<6} {status_emoji:<6} {nodelist}" --from-stdin < emojitest.txt

emojitest.txt

@trws
Copy link
Member

trws commented Oct 15, 2022 via email

@garlick
Copy link
Member

garlick commented Oct 18, 2022

Idea: add flux jobs -o cute support (a named format). I'm guessing there may be a user demographic that prefers emojis but eschews reading man pages? (It might include me)

@chu11
Copy link
Member Author

chu11 commented Oct 18, 2022

Idea: add flux jobs -o cute support (a named format). I'm guessing there may be a user demographic that prefers emojis but eschews reading man pages? (It might include me)

I like it! I could be the default output but with {status_emoji} instead of {status_abbrev}

@chu11
Copy link
Member Author

chu11 commented Oct 19, 2022

re-pushed, adding --format=cute per @garlick suggestion

image

@grondo
Copy link
Contributor

grondo commented Oct 19, 2022

Should the heading be just ST or maybe itself an emoji?

@chu11
Copy link
Member Author

chu11 commented Oct 19, 2022

Should the heading be just ST or maybe itself an emoji?

I did "STATUS" b/c the abbreviated headings were a little narrow for potentially wide emojis.

hmmm, an emoji as a header may solve alignment. Nothing comes to mind on the emoji list, could just do a ❓

@garlick
Copy link
Member

garlick commented Oct 24, 2022

I don't think this would hurt to merge as is since it's not a default output format and discussion has tailed off - if you're ready that is. Looks like there's at least a conflict to fix up

@chu11
Copy link
Member Author

chu11 commented Oct 24, 2022

I don't think this would hurt to merge as is since it's not a default output format and discussion has tailed off - if you're ready that is. Looks like there's at least a conflict to fix up

Yup, I'm good with it, lemme fix up that conflict.

Problem: flux lacks a certain cuteness factor.  This limits its
potential to be liked by younger HPC users.

Solution: Support the {state_emoji}, {result_emoji}, and
{status_emoji} fields, which will output the emojis representing job states
or results respectively.

Fixes flux-framework#4681
Problem: emoji output fields are not documented in flux-jobs(1).

Solution: Add {state_emoji}, {result_emoji}, and {status_emoji}
output fields.
Problem: emoji output tests were missing

Add some coverage in t2800-jobs-cmds.t.
Problem: In order to get emoji outputs in flux-jobs, users have to
use output fields in a format string.  Users need access to emojis
yesterday, they don't have time to learn how to format emojis.

Add a "cute" output format.  It's the same as the default output but
uses "{status_emoji}" instead of "{status_abbrev}".
@chu11
Copy link
Member Author

chu11 commented Oct 24, 2022

re-pushed fixing up conflict and a python isort issue that is now discovered

@codecov
Copy link

codecov bot commented Oct 24, 2022

Codecov Report

Merging #4687 (f47fc0e) into master (b884d04) will decrease coverage by 0.89%.
The diff coverage is 81.03%.

❗ Current head f47fc0e differs from pull request most recent head c3c8d9c. Consider uploading reports for the commit c3c8d9c to get more accurate results

@@            Coverage Diff             @@
##           master    #4687      +/-   ##
==========================================
- Coverage   84.24%   83.35%   -0.90%     
==========================================
  Files         410      413       +3     
  Lines       61515    69664    +8149     
==========================================
+ Hits        51826    58069    +6243     
- Misses       9689    11595    +1906     
Impacted Files Coverage Δ
src/cmd/flux-jobs.py 95.52% <ø> (ø)
src/bindings/python/flux/job/info.py 87.78% <81.03%> (-1.55%) ⬇️
src/cmd/top/ucache.c 86.66% <0.00%> (-13.34%) ⬇️
src/modules/job-info/allow.c 76.66% <0.00%> (-12.70%) ⬇️
src/common/libzmqutil/monitor.c 80.00% <0.00%> (-11.03%) ⬇️
src/shell/builtins.c 91.66% <0.00%> (-8.34%) ⬇️
src/shell/kill.c 92.30% <0.00%> (-7.70%) ⬇️
src/common/libutil/timestamp.c 92.85% <0.00%> (-7.15%) ⬇️
src/modules/resource/acquire.c 65.04% <0.00%> (-7.05%) ⬇️
src/cmd/builtin/dmesg.c 87.58% <0.00%> (-6.86%) ⬇️
... and 331 more

Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

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

LGTM!

@mergify mergify bot merged commit a45fe69 into flux-framework:master Oct 25, 2022
@chu11 chu11 deleted the issue4681_result_emoji branch October 27, 2022 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

flux-jobs: add {result_emoji} as a field
7 participants