Skip to content

Add cluster schedule source #50

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

stinovlas
Copy link
Contributor

@stinovlas stinovlas commented Dec 5, 2023

Similarly to broker and backend, I added a cluster version of redis schedule source.

The implementation is in separate class. I also duplicated the tests of RedisScheduleSource and applied them to cluster source. I also added a few more tests for both RedisScheduleSource and RedisClusterScheduleSource and the taskiq_redis.schedule_source module is now completely covered.

There is no documentation for schedule sources yet, so I didn't touch it in this PR, but I'd like to find some time to add documentation about cluster mode in general and schedule sources in particular to README.

@codecov-commenter
Copy link

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (caeccfb) 94.24% compared to head (4074e81) 93.54%.

Files Patch % Lines
taskiq_redis/schedule_source.py 88.88% 3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop      #50      +/-   ##
===========================================
- Coverage    94.24%   93.54%   -0.70%     
===========================================
  Files            7        7              
  Lines          191      217      +26     
===========================================
+ Hits           180      203      +23     
- Misses          11       14       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stinovlas stinovlas marked this pull request as draft December 5, 2023 10:18
@stinovlas stinovlas force-pushed the add-cluster-schedule-source branch from 4074e81 to 05cf89d Compare December 5, 2023 10:31
@stinovlas stinovlas marked this pull request as ready for review December 5, 2023 10:35
@stinovlas
Copy link
Contributor Author

Codecov report is not updated, my second commit actually covers all lines and increases total coverage to 96%.

@stinovlas
Copy link
Contributor Author

@s3rius Could you please take a look at this? Or perhaps delegate someone else to the review? I think this is the last piece missing to be able to fully utilize taskiq-redis in cluster mode :-).

Copy link
Member

@s3rius s3rius left a comment

Choose a reason for hiding this comment

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

Since this source almost completely copies the default redis schedule source, I don't se any issue with merging it.

Thank you for your contribution and sorry for late reply.

@s3rius s3rius merged commit 6268b15 into taskiq-python:develop Dec 12, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants