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

Add -M option for machine-readable JSON output #27

Merged
merged 19 commits into from
Oct 15, 2022

Conversation

bstee615
Copy link
Contributor

@bstee615 bstee615 commented Oct 3, 2022

This PR adds the -M short option which converts the output of ts or ts -l to JSON.
Also, I enabled CUDA build for an older CMake version (3.16) which is the latest by default in Ubuntu's package manager.

I implemented it with cJSON, which I added directly into the project. It is compatible with ANSI C so should not restrict any platforms.
I added a test to testbench.sh, not sure if any other test suite exists.

Please let me know if there are any other requirements for implementing this feature.

Example:

~/code/task-spooler-structured-output/build-gpu master*
❯ ./ts -K

~/code/task-spooler-structured-output/build-gpu master*
❯ ./ts echo foo
0

~/code/task-spooler-structured-output/build-gpu master*
❯ ./ts sleep 10s
1

~/code/task-spooler-structured-output/build-gpu master*
❯ ./ts
ID   State      Output               E-Level  Time   GPUs  Command [run=1/1]
1    running    /tmp/ts-out.vj9wHU                   0     sleep 10s
0    finished   /tmp/ts-out.tFVNpx   0         0.00s 0     echo foo

~/code/task-spooler-structured-output/build-gpu master*
❯ ./ts -M json
[{"ID":1,"State":"running","Output":"/tmp/ts-out.vj9wHU","E-Level":null,"Time_ms":null,"GPUs":0,"Command":"sleep 10s"},{"ID":0,"State":"finished","Output":"/tmp/ts-out.tFVNpx","E-Level":0,"Time_ms":0.0018889999482780695,"GPUs":0,"Command":"echo foo"}]

@bstee615
Copy link
Contributor Author

bstee615 commented Oct 3, 2022

By the way, could you add the hacktoberfest-accepted label to the PR if it is accepted so I can get credit for hacktoberfest? smile

@bstee615
Copy link
Contributor Author

bstee615 commented Oct 3, 2022

Implements #25

@justanhduc
Copy link
Owner

Hey @bstee615. Thanks a lot for your effort! This week I am quite occupied to prepare for my defense, so I will review soon within next week.
Btw, why -M while both -j and -J are available and seem to be a more natural choice?

By the way, could you add the hacktoberfest-accepted label to the PR if it is accepted so I can get credit for hacktoberfest? smile

Of course! Let me figure it out how to use it first.
Again, thank you for the PR.

@bstee615
Copy link
Contributor Author

bstee615 commented Oct 5, 2022

No problem about the time line, good luck on your defense.
I chose -M for "machine-readable", where the user can give options other than JSON. Currently tsp -M json gives JSON output, maybe later we would want to add tsp -M csv. I have no strong preference on that, so I can change it to a binary flag -J if you think it's better.

@justanhduc
Copy link
Owner

Hey @bstee615. Before merging, I would like to double check. Is the hacktoberfest label valid?

@bstee615
Copy link
Contributor Author

Yes, the label is valid. Thanks for the review!

@justanhduc justanhduc merged commit fbab5fa into justanhduc:master Oct 15, 2022
@justanhduc
Copy link
Owner

Thanks a lot for the PR @bstee615. Please let me know when the GUI is usable!

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.

2 participants