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 method subscribe_to #252

Merged
merged 15 commits into from
Dec 5, 2023
Merged

Add method subscribe_to #252

merged 15 commits into from
Dec 5, 2023

Conversation

WaYdotNET
Copy link
Contributor

Idea based from #236

I added a method that allows you to directly register a function that will then be "converted" to a celery task and will be automatically added to the topic so that you can improve the devX

@WaYdotNET WaYdotNET changed the title Add mehod subscribe_to Add method subscribe_to Oct 28, 2023
@WaYdotNET
Copy link
Contributor Author

cc @Mulugruntz @glujan

Copy link
Owner

@Mulugruntz Mulugruntz left a comment

Choose a reason for hiding this comment

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

@WaYdotNET
I tried to go around it to have subscribe that can be used both as a decorator and as a normal function (like it is today). But I think your approach (@subscribe_to vs. subscribe) is correct, better, and makes more sense, especially from a user perspective.
In the end, having this _to makes it more readable.

Thanks!

Tell me if you need more help :)

celery_pubsub/pubsub.py Outdated Show resolved Hide resolved
celery_pubsub/pubsub.py Outdated Show resolved Hide resolved
celery_pubsub/pubsub.py Outdated Show resolved Hide resolved
celery_pubsub/pubsub.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
…prevent unnecessary conversions by ensuring the Callable typing for the task is derived directly from task input.

 The function now immediately verifies if the existing function is an instance of Task. If not, it would convert the function into a Task instance. This modification simplifies the code and enhances execution efficiency.
… module

Expanded unit test coverage in test_pubsub.py with methods test_8 to test_11 to validate the functionality and expected results for the publish method with different parameters. Refactored job fixture methods in conftest.py to increase code readability. Now, the Callable typing for each 'job' is directly derived from the task input, enhancing code execution efficiency. New job fixtures (job_h to job_m) have also been added for additional testing purposes.
@WaYdotNET WaYdotNET requested a review from Mulugruntz December 3, 2023 09:42
Copy link
Owner

@Mulugruntz Mulugruntz left a comment

Choose a reason for hiding this comment

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

It's getting very close :)

Next step would be:

  • Update the README.md to add this new @ in the examples / docs.
  • And the changelog in the README.md

tests/conftest.py Outdated Show resolved Hide resolved
celery_pubsub/pubsub.py Outdated Show resolved Hide resolved
@Mulugruntz
Copy link
Owner

Hey @WaYdotNET !
As you may have noticed, I just improved the CI a bit. Now, it triggers on the PRs targeting master. This should help people contributing.

I'll still need to do some cleanup on this at some point. As the style that has accumulated over the years is a bit outdated.
I had to drop support for Celery 3, as it was now unable to run on GitHub Actions, even on 3.7 and 3.8.

celery_pubsub/pubsub.py Outdated Show resolved Hide resolved
@WaYdotNET WaYdotNET requested a review from Mulugruntz December 3, 2023 20:03
Mulugruntz and others added 5 commits December 4, 2023 08:13
…ntz#261)

* ci: Change the GitHub Actions trigger

Now, it will run tests for every pull request made to the `master` branch, as well as when merges are pushed to `master`.

* Drop support for Celery 3

Celery 3 depends on `use_2to3` which is no longer supported by any of the latest patch versions of CPython and Pypy, for >= 3.7.

The CI for Celery 3 was not able to run on any Python provided by GitHub Actions. It is time to fully decommission it.
remove Celery 3 info and add subscribe_to examples
# Conflicts:
#	.github/workflows/build.yml
#	README.md
#	tests/conftest.py
@WaYdotNET
Copy link
Contributor Author

WaYdotNET commented Dec 4, 2023

@Mulugruntz missing CC_TEST_REPORTER_ID in PR workflow ?
(https://github.com/WaYdotNET/celery-pubsub/tree/master is green :D )

@Mulugruntz
Copy link
Owner

Mulugruntz commented Dec 4, 2023

@Mulugruntz missing CC_TEST_REPORTER_ID in PR workflow ? (https://github.com/WaYdotNET/celery-pubsub/tree/master is green :D )

Can you rebase?

And also drop 5d92a26. I'll do the version bump and tag and publish later, all at once.

ci: Move CodeClimate ID from secrets to vars (Mulugruntz#267)
@WaYdotNET
Copy link
Contributor Author

some issue

@Mulugruntz
Copy link
Owner

Can you try again?

@WaYdotNET
Copy link
Contributor Author

Great!! it work well

Copy link
Owner

@Mulugruntz Mulugruntz left a comment

Choose a reason for hiding this comment

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

Looks good to me! 🚀

@Mulugruntz Mulugruntz merged commit f8674b3 into Mulugruntz:master Dec 5, 2023
18 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.

2 participants