Implement list_jobs method in scheduler#83
Conversation
Added a method to list names of all registered jobs.
There was a problem hiding this comment.
You need to add tests for this. Usually there should be a test_scheduler but it seems like it was skip when we worked on that so it's a good opportunity to add it. It will be useful for the #71 Otherwise, it looks good to me.
Add tests for Scheduler's list_jobs method to verify it returns an empty list and proper format.
|
Hey @PenguinBoi12! I have switched branches to patch-1 and successfully created the missing test_scheduler.py file inside the tests directory to verify the list_jobs logic. Could you please approve and trigger the workflow tests to verify the suite when you get a chance? |
You should run the tests, mypy and black locally before pushing to make sure the workflow will pass:
|
PenguinBoi12
left a comment
There was a problem hiding this comment.
We need to have a test that validates that pour list works correctly.
You didn't follow the projects formatting, you can run black . to format the changes you've made. Without that the CI won't pass.
There was a problem hiding this comment.
Removing the test is not an appropriate way to fix the issue. Now the feature isn't fully tested.
|
Hey @Shouryaverma19, I see that you tried to fix the formatting. The easiest what is to run Additionally, you did not address the issue with the tests. We should have a test that actually validate the result when jobs are listed. Thank you and if you need anything let me know. |
Removed duplicate list_jobs method.
Add method to unschedule jobs by name.
Added a sample task function and expanded the test to include job management scenarios.
Added a method to list names of all registered jobs.