Add an option to work without Celery?

[beatonma]

Add an option to work without Celery?

[beatonma]

Hey, just want to update you on this - I got a bit sidetracked while making the testing tool I mentioned. Back to this...

Based on recommendations from @GriceTurrble in #19 #20, I have just published a version 1.3.1. It doesn't change anything functionality-wise but just adds a warning for existing users that migrations will soon be included and that this will be a breaking change.

Next step will be to publish version 2.0.0. This will include the migration files, and bump the recommended celery version due to a recent advisory. Both of these are potentially breaking changes so think it makes sense to bundle them together.

Finally, I will publish version 2.1.0 which will include the changes you requested for bypassing celery. Hopefully this will happen in the next day or two - sorry it's taken so long!

[beatonma]

Hey, sorry for the late response. Good idea! You are absolutely welcome to work on this and I'd be happy to accept any pull requests that make it more useful/usable.

I started thinking about how I would approach this yesterday and ended up just starting it - https://github.com/beatonma/django-wm/compare/without-celery - basically using the approach you suggested.

The commit is messy but basically:

• Added settings.WEBMENTIONS_USE_CELERY
• Bypass any existing celery imports if it is not installed.
• Added PendingIncomingWebmention and PendingOutgoingContent models.
• Added pending_mentions management command to process any instances of those models.
• Replaced celery <function>.delay() calls with handle_incoming_webmention and handle_outgoing_webmention (See here) which handle passing to celery or creating Pending... model instances.

I ran out of time so this is completely untested and needs some tidying up but it should be close to working..? Feel free to use this as a starting point if there is anything else you want to add or fix.

[philgyford]

Oh this looks great, thank you! Probably nicer than whatever I'd have attempted. I'll try and give it a whirl in the next few days and see if I can think of anything else to do with it. Thanks again.

[philgyford]

Sorry I didn't get a chance to look at this sooner - Christmas, New Year, etc got in the way.

I've only had a quick go so far, but some initial questions:

1. Is there a reason why the app doesn't have any migrations? I know I'd have to create a migration for my own app after adding MentionableMixin to a model, but it seems very unusual for there to be no migrations for the non-mixin models.
2. There's a typo in tasks/scheduling.py on line 14 - it should be WEBMENTIONS_USE_CELERY not WEBMENTION_USE_CELERY (no S).
3. It would be useful to have Admin classes for PendingIncomingWebmention and PendingOutgoingContent so it's possible to see what's waiting to be dealt with. Otherwise it's hard to know if things are working.
4. I don't think there's anything that prevents duplicate mentions being sent. This can happen (a) if a text links to the same URL more than once, or (b) if an object is saved more than once. I guess this should happen in tasks.outgoing_webmentions.process_outgoing_webmentions()?

[beatonma]

No problem, same deal with festivities here.

1: Not really. At the time of original release I had seen discussion on whether migrations should be included in git and decided it made sense for the consumer to make their own migrations. I have since learnt the error of my ways. Adding them now may complicate things for any existing users but it's probably the right thing to do overall.

2, 3, 4a: Thanks, will fix.

4b: This was intentional so that the mentioned site can know that the source content has changed for moderation purposes. Of course there's nothing stopping you from changing the content quietly anyway, but that's the intent for allowing duplicates. That said, it may be good to add some sort of checks on incoming mentions.

I'm going to work on this a bit this week. I'm also planning to create a testing tool on my main site - I took down the django-wm.dev testing site a while ago as it wasn't being used much, but it would be handy to have that functionality available again.

[philgyford]

Brilliant, thanks! I was hoping to have some time to make the obvious fixes (2, 3) myself but work got in the way. I also haven't worked out how best to use a fork of a third-party app within the local dev version of my site, so it's easier to make changes for a PR.

Yes, you're right about 4b, despite having worked through the Webmention spec some time ago I forgot that it's good to send again in case anything changes. But then, as you say, the onus is on the incoming side to handle any duplicate/updated webmentions appropriately.

[philgyford]

No problem at all! Thank you so much for doing all this!

I did post a hypothetical question to r/django about how one would add migrations to a package if it was already being used by people. @GriceTurrble asked me in chat what the package was, so thanks to him for offering such clear advice too!

❤️

[beatonma]

2.1.0 is now available and the testing tool is now up and running. Updated setup instructions (without celery) start here. Let me know how you get on :)