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

Cleanup after unloading extension. #3226

Merged
merged 6 commits into from
Jul 15, 2023

Conversation

Jerrie-Aries
Copy link
Contributor

This resolves #3223.

  • All files and modules in plugin's directory will be properly unloaded after unloading the plugin. This is done internally when using ?plugin remove ... or ?plugin update ... command.
  • Fix files in plugin's directory were not re-imported after updating the plugin.

Generally, this is not a breaking change. Existing plugins would still work just fine.

Reference:

@Taaku18
Copy link
Collaborator

Taaku18 commented Mar 14, 2023

Is this necessary? Deleting from sys.modules tends to cause problems. We suggest always restarting your bot after updating / removing plugins.

@Jerrie-Aries
Copy link
Contributor Author

Jerrie-Aries commented Mar 14, 2023

I'm not sure about the issue this will cause. But discord.py itself does this in the background when an extension is removed. It's just our folder structure for plugins is different from what is expected from discord.py that makes the imported files from plugin folders were not entirely removed. You can see my comment here.
The only possible issue I can think of is when files from a plugin directory were imported from another plugin. But that is rare case, right?

Is this necessary?

For QOL improvements, I think this is necessary.

I also read on dpy server (a while ago before creating this PR) when someone asked about the flexibility of removing modules when unloading extensions, Danny said it should be handled by developers (not library).

Copy link
Collaborator

@Taaku18 Taaku18 left a comment

Choose a reason for hiding this comment

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

Everything looks good except one minor issue. Once that issue comment gets resolved, we can proceed with the merge. :)

cogs/plugins.py Show resolved Hide resolved
Taaku18
Taaku18 previously approved these changes Jul 14, 2023
@Taaku18 Taaku18 added staged Staged for next version and removed in progress labels Jul 14, 2023
@Taaku18 Taaku18 merged commit a784f82 into modmail-dev:development Jul 15, 2023
@Taaku18 Taaku18 mentioned this pull request Jul 15, 2023
@Jerrie-Aries Jerrie-Aries deleted the dev-issue-3223 branch July 15, 2023 13:57
raidensakura pushed a commit to raidensakura/modmail that referenced this pull request Apr 10, 2024
* Cleanup after unloading extension, resolve modmail-dev#3223.

* Remove leftover modules loaded from `plugins` path when purging.

---------

Co-authored-by: Taku <[email protected]>
khakers pushed a commit to khakers/OpenModmail that referenced this pull request Jun 18, 2024
* Cleanup after unloading extension, resolve modmail-dev#3223.

* Remove leftover modules loaded from `plugins` path when purging.

---------

Co-authored-by: Taku <[email protected]>

(cherry picked from commit a784f82)
Signed-off-by: Khakers <[email protected]>
khakers pushed a commit to khakers/OpenModmail that referenced this pull request Jun 18, 2024
* Cleanup after unloading extension, resolve modmail-dev#3223.

* Remove leftover modules loaded from `plugins` path when purging.

---------

Co-authored-by: Taku <[email protected]>

(cherry picked from commit a784f82)
Signed-off-by: Khakers <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
staged Staged for next version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]: Updating or reloading plugins does not reload imported files.
2 participants