Update Job.get_status and Job.restore to consistently return JobStatus Enum #2039
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2039 +/- ##
==========================================
+ Coverage 93.61% 93.76% +0.14%
==========================================
Files 28 29 +1
Lines 3758 3863 +105
==========================================
+ Hits 3518 3622 +104
- Misses 240 241 +1 ☔ View full report in Codecov by Sentry. |
rq/job.py
Outdated
| @@ -322,18 +322,18 @@ def get_position(self) -> Optional[int]: | |||
| return q.get_job_position(self._id) | |||
| return None | |||
|
|
|||
| def get_status(self, refresh: bool = True) -> JobStatus: | |||
| def get_status(self, refresh: bool = True) -> Optional[JobStatus]: | |||
There was a problem hiding this comment.
I think get_status() should always returns something. If the job hash in Redis has no status, raise an exception.
There was a problem hiding this comment.
Thank you for the note. I updated it to raise an exception if there's no status in Redis. I added a status=JobStatus.QUEUED to a few Job.create calls in other tests so that they wouldn't fail as a result. I don't think this changed the purpose or accuracy of those tests.
| Returns: | ||
| status (JobStatus): The Job Status | ||
| """ | ||
| if refresh: | ||
| status = self.connection.hget(self.key, 'status') | ||
| self._status = as_text(status) if status else None | ||
| if not status: | ||
| raise InvalidJobOperation(f"Failed to retrieve status for job: {self.id}") |
There was a problem hiding this comment.
Mind updating the test to test this code path so it can pass Codecov's test?
|
Thanks! |
I wrote up what I found in issue #2038, so I though I should create a PR with my suggested fixes.