Red-discordbot: Remove hot reloading from production

[zephyrkul]

As it stands, hot reloading cogs comes with several issues that can heavily impact end users; quite a few issues in cog support channels come from users hot reloading cogs and needing a full bot restart.

Given that end users rarely need to hot reload cogs, and that for most bots, restarting has little impact, it may be worth removing hot reloading from production code entirely. If the (quite complicated) issues regarding hot reloading ever get resolved, it can be returned to production code.

However, cog creators, developers, and some power users will still find heavy use out of hot reloading for development purposes. Thus, should reload be moved out of production code, it should remain an option inside the Dev cog, which is meant for providing an easier environment for testing, etc.

[Kowlin]

If we're gonna move on with removing it from production, I'd strongly recommend making it an optional arg var.

[fixator10]

I have about 80 cogs on my instance and not really noticing any problems with hot reload. (Except, perhaps, a ton of Warnings on V3/develop…) Hot reload is very useful feature even on update, so im not sure if that really good idea...

As possible feature, maybe add an ~hot_reload attr to cogs or info.json if cog is complicated and requires restart to update, and do not hot reload these cogs on update.

If there is really critical issues with hot reloading, perhaps they should be fixed either in cog's structure or in core's reload logic

[Stonedestroyer]

If hot reload is removed what stops people from using unload and load in sequence? I haven't checked out the code but pretty sure reload unload and just loads.

[Flame442]

The major issues with hot-reloading stem from how Python handles references when we try to reimport a cog. It's not something that is easily fixable despite a lot of effort by devs to find a workaround. Any cog is potentially at risk of having a hot-reload issue (after a change from something like [p]cog update especially), however most reloads will not have any issue.

The average user (and most people who would be commenting on this forum are considered power users) most likely only reloads cogs when they are a) not functioning or b) after a cog update. Since they do not need to reload very often, it is less of a burden for them to restart their bot the few times they do reload cogs. However, moving [p]reload to the dev cog (or adding a cli flag enabling the command as kowlin suggested) keeps the functionality for power users and cog/core developers who are more knowledgeable about the potential dangers of hot-reloading.

One potential downside to this change is that it may make cog/core support a little bit harder, rather than telling a user to just reload audio if lavalink dies, they may have to restart the entire bot, a more troublesome experience. I personally believe issues like this are rare enough for the average user that needing to restart their bot once in support is not a detrimental experience, however I would love some feedback on any potential negative impact this (or any other cons) could cause.

As discussed in discord for a little bit, this would probably involve limiting cogs to only be "loaded" once per bot launch, and if a cog is unloaded, the bot would need to be restarted to load it again. This ensures users can retain some freedom in their ability to load new cogs (through downloader, etc) while still ensuring that hot reload issues do not happen. If possible, we could load the already imported version of the cog so the user does not have to restart after unloading a cog, however that may cause other complications.

[mikeshardmind]

A word of caution. The issues with Red's hot reload are fixable and are an issue with Red. Discord.py reload extension does not experience these issues.

[jack1142]

Let me start with saying, despite what some people might think, I'm against removing hot-reload for end-users from the project.

The modularity of Red is one of its key features and in my opinion shouldn't be treated lightly. The option of removing hot reloading from production means that we're not taking this useful feature from cog developers, but it still takes away some of the ease of use from end users. The feedback from the latter, we won't really get on this issue as most of them doesn't keep an eye on issue tracker or #v3-burndown. I don't have a good way of getting feedback from them, but we really need to consider what it means to end users very thoroughly.

But before even considering dropping this feature, I think what we should start with is, what exactly are the issues with hot-reload. That way we can decide whether it really should come down to ditching a quite useful command. And I don't ask people here for possible solutions (or lack of them) to those issues cause that requires more time to figure out, it is a quite complex issue and it's not easy to deal with, especially now that we are short of people that are capable of fixing such issues. But if you have some solution to propose, feel free to mention it.

Issues that I know of are:

• cog updating a version of library it depends on - Red doesn't reload libraries cog imports
• something around singletons, I know of one such issue in Downloader, but it's not really clear what exactly makes it break, so I encourage people here to give more examples that break singletons (and if possible, making them into smallest reproducible example to make it clear what it really is about)

I really encourage all feedback about issues on hot-reload here cause whatever we choose do later with this can strongly depend on this.

[yamikaitou]

I've heard about it mostly in passing, but I agree with Jack that we should document what exactly the issues people are encountering with hot-reload. How often does it happen? Do we kind of know the exact cause? What group of users are more likely to encounter the issue? If it is truly a rare situation to encounter, can't we just live with it while exploring the fix? I think the more people understand the issue, the better people can either voice their opinion, help find a fix, think up a workaround, etc.

Ultimately, I do not agree with the complete removal of the ability to reload cogs. If anything, hide it behind a flag or within the Dev cog (although I'd rather it stay as is). We can't remove something that is so fundamental to the concept of Red without it being a last resort.

[mikeshardmind]

As already noted, the issues with Red's hot reload are solely a red issue and can be fixed.

The timing of people actually willing to talk about this issue is quite absurd, and the issues with hot reload are well documented in a discord channel where prior attempts to get people to discuss handling it went mostly without response.

Since there is visibility on this issue which exceeds that of the channel where that happened, I'll reiterate what I already pointed out there.

1. Red's reload logic is wrong and needs fixing.
2. Even upon fixing it, it interacts with another issue of dependencies being updated.
   
   
   
   • This particular case (and no other) do need special handling.
     
     Right now, there is no logic that tracks which dependencies might conflict between cogs or between cogs and core. Fixing this would allow the downloader cog to know when a restart was needed or when a cog might be incompatible with another cog.
   
   

[jack1142]

@mikeshardmind While you are right that the issues with hot reload have been discussed in discord internally, gathering it all from small parts of various discussions is not that easy, there were different problems brough up, some solutions proposed, but what I am trying to do here is gathering it all in one place so that we can actually look at options that we have here that are in our reach.

The timing of people actually willing to talk about this issue is quite absurd

I really don't want to get into this, but in short, with our team losing one (and only one that had time to contribute to Red) of core maintainers (yes, I do mean you but what matters here is that we just don't have highly experienced people that can help us), we now need more help from outside, cause we're lacking active and experienced people that could handle this stuff. People have their lifes and this is an open-source project so we do not blame anyone for that, but we're still trying what we can to keep the project alive and in good state.

[mikeshardmind]

I really don't want to get into this, but in short, with our team losing one...

I have no desire to get into a fight about it, but I have my reasons for finding the timing absurd. If you'd like to discuss this (discuss, not fight or argue), my DMs are open.

I've summed up what I'm aware of as problems above if you'd like a concrete example of where Red does the wrong thing and discord.py handles it correctly, __init__.py of a module doesn't get updated properly in Red. #2446 is still a suitable fix for this and other issues, leaving only the separate but related dependency issue I mentioned. Just because that's a suitable fix, does not mean there may not be cogs not taking advantage of things such as teardown functions to behave properly.

gathering it all from small parts of various discussions is not that easy

As for summing up potential solutions and the pros/cons which were raised, I can do that from memory without access to the channel.

• Status Quo, the issues are rare, just tell people to restart if they run into an issue.
  
  
  
  • (-) Leaves a fixable issue unfixed.
  
  
  • (+) Low effort, low impact.
  
  
• Handle multiple version of dependencies by import hook
  
  
  
  • (-) Will break on lazy imports
  
  
  • (-) Convoluted and hard to maintain
  
  
  • (-) May lead to issues around module-level constants
  
  
  • (+) Allows shared libs to be un-deprecated
  
  
  • (+) Allows more user choice
  
  
• Install dependencies into a venv owned by the downloader
  
  
  
  • (-) Requires passing all of Red's dependencies to each install command to ensure lack of conflict (pip is not a full dependency resolver)
  
  
  • (+) Allows wheels to work properly (see NumPy dependant cogs on ARM...)
  
  
  • (+) Prevents re-installing dependencies that already are installed.
  
  

[Dav-Git]

I'm not sure if feedback on this is still wanted, but since it was brought up that everyone commenting here is considered a Poweruser I asked a friend of mine who uses a bot that I host for them and take care of all the technical aspects. He usually goes through cogs rather quickly, meaning that he installs and uninstalls them all the time based on what he needs in the moment. If we were to limit cogs from being unloaded and loaded again without a bit restart that would completely kill his usecase for the bot. He's also very afraid of doing restarts out of fear that something might stop working, which is why I always do them when they are needed. He's pretty comfortable with [p]cog update though and does most of that stuff on his own, in the process hot reloading his cogs.

I know this is only one end user but I think it's something worth mentioning. Personally I'm in favor of keeping the hot reload as well. And if it takes time to fix it, so be it, but it's one of red's key features that should in my opinion not be shaved of completely.

[Twentysix26]

@zephyrkul I think this should stay open. I'd prefer not to make a new issue about this, there's valuable input here that would be lost. You can unsubscribe from this issue if you prefer not to be bothered by new messages notifications.

[Tobotimus]

I've always thought of hot-reload as a somewhat "dodgy" feature, even though it seems to work most of the time, sometimes it may cause problems you're not really aware of, and it wouldn't be obvious that the fix is just to restart your bot (in most cases; we're yet to see a case where a partial reload corrupts someone's config!).

However despite this I'm actually in favour of keeping it per #2446, as well as adding a warning to it, saying something like "should you experience issues with these cogs, try restarting your bot".

I will say now that we will never fix hot-reloading of dependencies, however that is another discussion entirely since our dependency management system probably has bigger problems to be taken care of first.

[zephyrkul]

@zephyrkul I think this should stay open. I'd prefer not to make a new issue about this, there's valuable input here that would be lost. You can unsubscribe from this issue if you prefer not to be bothered by new messages notifications.

My apologies, I had already done so but the issue still appeared at the top of my GH homepage. I won't close it again.

[skylarr1227]

I want to give a public thank you to to you @mikeshardmind for all you've done with red, as red has not only introduced me to python as a fun hobby-but also been something of great benefit-in so many ways to me and my bots users... This is a fraction of what you deserve in my opinion but unfortunately its all got. Thanks<3

I know I am not a "Cog creator" but this makes me very sad to witness because I dont think this project would even be discussing this issue if it was not for Sinbad and his contributions(then again, i could be wrong Im just giving an outsiders opinion).

Now with that out of the way- I will explain this issue from the viewpoint of a long time user of redbot- I've now been using redbot for over a year now and as I saw others mention here, I think that while still keeping this function by use of args or whatever other optional means is better than not fixing the issue and leaving a problem (ive never had any issue with hot reloading.. and trust me with how bad I am at python-I am always updating and reloading like hundreds of times a day) this is not a solution to a problem... This is a way to push a problem to the side, because its easier or not as time consuming to do so and the majority of people then would not have a problem to report because it would not be something most used(i still dont believe that, this functionality isn't just a small part of redbot).

I would say in my opinion-what keeps me using redbot and what I see as its most valuable "assets" can be summed up in 3 different "functions" and 1 "ideal?" idk what to call it:
(in order from most useful to least)

1. Insanely robust cog system-allowing each redbot instance to be as insanely unique and customized as the owner wants or has time for-especially if the owner is capable and willing to make cogs. Included in this-IS HOT RELOADING of cogs. This blows me away, as this is part of reds biggest and best features, I would hate restarting my bot more, and that would require a LOT more restarting if this did not exist... were not talking about dropping the audio cog here this is big.
2. Total reliability-if the proper steps outlined within each cog creators readme, and the redbot documentation are followed, the core of redbot is insanely durable and hard to crash if possible without --dev functions. Mine has 24/7 up time for the past year of around 97.77%. (my host did maintenance and had issues for 2 days.)
3. Instant support from crazy good programmers. That being said-more than one person within the redbot team and their attitudes towards people have more than once nearly caused me to quit using it-but over all devs like @mikeshardmind have kept me using this.

Now this being said-I know most likely my opinions dont matter, but I felt this topic was one worth sharing my opinion on because I feel this is the most valuable feature of the core redbot and if I didnt share and it got removed or pushed to the side where it could be problem that myself or others would have to deal with-would certainly mean me just moving away from redbot entirely. The guarantees outlined in the documentation have led me to believe core features would always remain supported and working-and i could be wrong here, seems like a way to avoid fulfilling this guarantee.

PS.
Sinbad-I hope one day you update your picture. much<3

[DevilXD]

I vote in favor of not removing hot reloading. As other people have said, it's a cool feature to have. It has it's own issues, but these can be solved with some dependency tracking.

[TrustyJAID]

I'll leave my note here as well that hot-reload should not be fully removed from the bot. There's a lot of instances where a cog is simple enough that hot-reloading will cause no issues at all and is incredibly useful for cog development.

In cases where it does cause issues, I feel, the owness is upon the cog creator to ensure that all the right things are in place within their own cog. It may be worth investing time in adding the warning to the user stating the user should restart when a dependancy has updated or have an option for cog creators to bypass said warning should their cog not have anything that could cause hot-reloading issues.

[jack1142]

I think we can close this now, we gathered a lot of feedback here and we resigned on the plan to remove hot-reload from Red in any foreseeable future (which probably means never, at least in V3).
And closing the issue doesn't remove the feedback so anyone can always come back to this ;)