-
Notifications
You must be signed in to change notification settings - Fork 4
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
Depend less on Celery #4
Comments
The current way of handling multiple hubs works, but it's a bit of a hack. It would be better to define the tasks only once. The overhead of passing in hub information with each job is probably worth it (especially after the changes above reduce the amount of required jobs). |
I've created a proof-of-concept which removes Celery as a run-time dependency. Instead, I've created a stub 'Celeriac' which is API-compatible with Celery, but simply uses an asyncio event loop in a separate thread. As the main code depends heavily on the Celery API, it was a bit of an undertaking to stub out unnecessary calls and replace needed calls with an asyncio equivalent. Of course, by doing it this way, I haven't introduced a proper abstraction facade yet: it's all still a bit "celery" (pun intended). |
Sounds interesting! Being able to swap out Celery for something else altogether is a bit wider in scope than the issue discussed here, but I would love for Flask-WebSub to support that. |
All the tests pass, but I already have butchered the pytest fixtures and simplified the tests, mainly because the TestApp code and the dependency on app _state and Celery workers are not necessary for Celeriac to function. Fortunately, I still have an old baseline with the test stubbing, from the time when I was still using the original testing code. Regarding that test stubbing, it suffices to say "pretty it ain't." I will go through the WebSub.rocks tests, next week, to find any remaining issues. It would be lovely to see this code end up in your package, and ultimately in PyPI. Also worth noting is that Werkzeug has changed its API, and the current baseline is not compatible with those changes. |
Okay, I have completed the WebSub.rocks Hub tests, successfully. However, I had to patch the hub/tasks.py code, as it doesn't seem to build up the request headers for a notification in a clean way. The following statement copies in a lot of headers, which do not belong in the notification request:
As a result, the WebSub.rocks server refuses the notification, on account of those stray headers:
I still need to stub out the Celery retry code, which I do not support yet (this became visible with the above tests). |
Weird, that's never been a problem before. |
The hub currently schedules a separate celery job for each HTTP request that it sends out during content delivery. That does not scale, especially with a large message body. A better approach would be to store the content in the database, and keep track of the last delivery for each subscription. Then a single background task can handle the content delivery.
The text was updated successfully, but these errors were encountered: