I'd like to add an autoreload_stopping Signal() to the autloloader

Hello. I’m new here but I’ve been using Django since version 1.x

I have a problem with the autoreloader and Threads.

I initially solved this issue (quite neatly I thought) with Added autoreload_stopping Signal to complement autoreload_started by lbt · Pull Request #16674 · django/django · GitHub

However that was not immediately well received so here I am :slight_smile:

I’ve created ticket #34432 (autoreloader does not reload when a Thread is running and there is no way to notify the Thread to stop) – Django to provide a definition, minimal case and a foundation for discussion

The patch I have proposed:

  • Is trivial and trivially maintainable
  • Has no performance issues
  • Is aligned with the existing design and use of a Signal() for use in internal code
  • Is also symmetric with the starting Signal()
  • Fits with the ‘at a distance’ principle of when Signals are a good design pattern
  • Is only applicable during development

Essentially the autoloader is about to forcibly sys.exit() and it seems reasonable to politely announce that fact and using the Signal framework seems reasonable and consistent with existing usage.

I am not aware of a better way to stop that Thread when the autoader is about to die :smiley:

This kind of comment isn’t helpful. We’ve merely asked you to follow the established process when requesting features which is especially important when asking for something like a new signal (for which it’s much preferred to steer people to other/better solutions).

You still haven’t explained why what you’re trying to can’t be achieved some other way.

The sys.exit() that you want to signal on is for the child process run by the autoloader — that exits completely, including any threads, so it’s not clear (not yet explained) what clean up needs to be done.

It may be that given how you’re doing it you want this approach, but likely there are other ways.

I hope that makes sense.

Erm - I’m sorry - it was just a light and factual comment that you didn’t exactly jump for joy and welcome the patch. Nothing more - it even said “so here I am with a smile” !
I personally found your responses quite encouraging and welcoming (even though they set my expectations that this was not an easy case to make).

So hopefully you can see that I completely accepted your suggestion to come here to discuss it and spent quite some time putting together a minimal test case, prepared a ticket and trying to summarise the arguments (as in the logical rationale, not emotional) for the patch.

To my mind the ticket provides a trivially reproducible case that shows that the child does not exit completely and the loader hangs waiting for it.

1 Like

As I re-read I’m wondering if what you mean by “You still haven’t explained why what you’re trying to can’t be achieved some other way” is you’re actually looking for more details of my application design and why I’m even using a Thread helper there?

Whereas for this PR I’m trying to minimise and explain the generic case of “If I start a Thread then I need to be able to stop it” which I think is a reasonable (though rare) undertaking in the AppConfig.ready() method.

Yes, exactly. I’d like to understand your use-case. That way it’s possible to say of what you’re doing is something we’d want to add code to Django to enable (or not)

Thanks.

OK I’m using Django as the ORM behind a home automation system.

I have multiple clients which display and act upon data changes and other events.

I am using MQTT as the message distribution system. When any client makes a change to the DB it issues an MQTT message and the other clients reload the DB record and update their displays or otherwise may act upon the change.

(Out of interest the standalone clients are actually running Qt’s Pyside with the qasync asyncio event loop and use SyncToAsync.single_thread_executor to manage their async Django usage. Not terribly relevant but I thought I’d mention it. :slight_smile: )

I’m using gmqtt (which is a fully async mqtt client) as the underlying MQTT library.

So in the Django context I start a gmqtt client in its own Thread with an asyncio loop.

I then use the post_save.connect() to publish using asyncio.run_coroutine_threadsafe() from a transaction.on_commit(). I could have used a specialised Model with a publish-on-save but instead the signal receiver checks the Model for an MQTT_publish_msg attribute and only publish if that is present.

The standalone clients initialise the MQTT client thread as part of startup and, of course, shut it down cleanly.

However I also provide a traditional web view onto the system and this is where the problem lies.

For the web view I ensure that a single MQTT server Thread is started in AppConfig.ready() and I then close down using atexit() which works well.

However when running with the development server I encounter the problems I set out in the ticket - autoreload doesn’t work.

So in all of this it’s just an annoying niggle in the web development environment but when I looked into it I realised that there is no way at all to do any generic cleanup before the reloader’s sys.exit() and I really was (and still am) surprised that this wasn’t just an “oh yes, it obviously makes sense to have a corresponding autoloader_stopping Signal()”.

Hey @lbt. Thanks for the follow up. I can see how this works.

Can I ask, what happens to the client thread when the sys.exit() is hit? How does not-reloading look?

How are you dealing with this currently? Monkey-patching trigger_reload()?

It might well be that’s the best solution: the use-case is quite niche, and adding a new signal to the public API for it might not be worth it. (There have been, though, various discussions about code that runs at start-up… so :person_shrugging:)

I need to have a play with this. Let’s see what others say.

Kind Regards,

Carlton

I’m currently running a patched version of Django in my venv. The code is such that the try: import is only hit in a development environment

I put some details and the minimal case in the ticket … #34432 (autoreloader does not reload when a Thread is running and there is no way to notify the Thread to stop) – Django
Does that help?