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 --worker and --query option in CLI #171

Merged
merged 2 commits into from
Sep 13, 2024
Merged

Add --worker and --query option in CLI #171

merged 2 commits into from
Sep 13, 2024

Conversation

gpetretto
Copy link
Contributor

Closes #170 adding the --worker and --query options in the CLI to filter based on worker name and with a custom query.

I considered adding the --query for filtering on flows as well, but it seems that the current options basically cover the content of the Flow document. However it can be added if it seems useful.

@codecov-commenter
Copy link

codecov-commenter commented Sep 4, 2024

Codecov Report

Attention: Patch coverage is 20.00000% with 20 lines in your changes missing coverage. Please review.

Project coverage is 47.86%. Comparing base (96baad0) to head (496e9b0).

Files with missing lines Patch % Lines
src/jobflow_remote/jobs/jobcontroller.py 8.33% 9 Missing and 2 partials ⚠️
src/jobflow_remote/cli/job.py 0.00% 5 Missing ⚠️
src/jobflow_remote/cli/utils.py 25.00% 2 Missing and 1 partial ⚠️
src/jobflow_remote/cli/types.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #171      +/-   ##
===========================================
- Coverage    47.94%   47.86%   -0.09%     
===========================================
  Files           43       43              
  Lines         5095     5106      +11     
  Branches      1103     1109       +6     
===========================================
+ Hits          2443     2444       +1     
- Misses        2395     2402       +7     
- Partials       257      260       +3     
Files with missing lines Coverage Δ
src/jobflow_remote/cli/types.py 96.07% <75.00%> (+5.33%) ⬆️
src/jobflow_remote/cli/utils.py 30.76% <25.00%> (+0.03%) ⬆️
src/jobflow_remote/cli/job.py 20.43% <0.00%> (-0.16%) ⬇️
src/jobflow_remote/jobs/jobcontroller.py 35.52% <8.33%> (-0.36%) ⬇️

Copy link
Member

@davidwaroquiers davidwaroquiers left a comment

Choose a reason for hiding this comment

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

LGTM! Just a few small comments. Maybe @janosh you can also have a look and test to see if it fits your needs ?

src/jobflow_remote/cli/utils.py Show resolved Hide resolved
src/jobflow_remote/jobs/jobcontroller.py Show resolved Hide resolved
src/jobflow_remote/jobs/jobcontroller.py Outdated Show resolved Hide resolved
@janosh
Copy link
Collaborator

janosh commented Sep 13, 2024

thanks a lot @gpetretto for implementing! looks great 👍

maybe worth mentioning the new flags in the dealing with errors docs?
perhaps --worker could be mentioned after this

If, for example, this was due to a problem with the connection to the worker that has already been resolved, it is possible to avoid waiting for the job to be retried. This can be done by running:
...
[To rerun all REMOTE_ERROR jobs for a given worker, use ...]

@gpetretto
Copy link
Contributor Author

Hi @janosh, thanks for the suggestion. I do not fully agree with it though. I feel like that the error section of the documentation should try to stay focused on the different cases that could happen concerning the errors. This would indeed add additional information, but more on a use case of the CLI rather than explaining how the errors are different from one another and which specific command to use. Maybe a new section of the documentation showing the more commonly used options for the CLI could be a better place to mention this? (I would need to leave this for a future PR though)

@janosh
Copy link
Collaborator

janosh commented Sep 13, 2024

Maybe a new section of the documentation showing the more commonly used options for the CLI could be a better place to mention this?

sounds good! i think such a page could make extra sense once the coming --count feature from #110 is implemented

@gpetretto gpetretto merged commit 90e42fd into develop Sep 13, 2024
5 checks passed
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.

Add --query (or --worker) flag to jf job rerun/retry
4 participants