Skip to content

Added the ability to choose the serializer#283

Open
amks1 wants to merge 3 commits intorq:masterfrom
amks1:job_serializer
Open

Added the ability to choose the serializer#283
amks1 wants to merge 3 commits intorq:masterfrom
amks1:job_serializer

Conversation

@amks1
Copy link
Copy Markdown

@amks1 amks1 commented Feb 20, 2023

RQ's Job object can be created with a custom serializer.
The _create_job() function and its invocations have been updated to use this kwarg.

@selwin
Copy link
Copy Markdown
Contributor

selwin commented Feb 24, 2023

Mind fixing the conflict? @amks1

@amks1
Copy link
Copy Markdown
Author

amks1 commented Feb 25, 2023

Mind fixing the conflict? @amks1

sure, done.

@selwin
Copy link
Copy Markdown
Contributor

selwin commented May 2, 2023

@amks1 the tests also need to pass before I can merge this in :)

@robhudson
Copy link
Copy Markdown

fwiw this same test fails locally for me on the master branch. It has something to do with timezone handling, as far as I can tell. I'm curious if it fails for others?

  • The test creates a job via a cron string to run 5 minutes after the hour. It is using freezegun.freeze_time to set the time to 00:00:00 when scheduling the job
  • then using a freeze_time context again to set the time to 00:05:00 and calls self.scheduler.enqueue_jobs()
  • it expects to find the job queued but fails

For me this fails since the get_next_scheduled_time returns a timezone aware datetime, whereas the others do not. So for me the next scheduled time is actually 07:05:00 due to my local timezone but it is looking for jobs scheduled to queue in the time context of 00:05:00 so it finds no jobs.

In my opinion this test should fixed up on the master branch, then this PR rebased.

@selwin
Copy link
Copy Markdown
Contributor

selwin commented May 31, 2023

It looks like the tests in master also fails, but what's weird is that tests seem to pass on Pythons 3.6, 3.10 and 3.11. ]\https://github.com/rq/rq-scheduler/actions/runs/4869468245/jobs/9248482531

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.

3 participants