reload: fix issue with reloading modules within subpackage #1398
Conversation
Force-reload all sub-modules of packages recursively. See sopel-irc#1056.
sopel-irl#1314 used recursion to reload nested modules. However, in Python 2 (maybe python 3 as well), this approach leads to odd behavior with callables being unloaded/reloaded twice. The loop detects my_module.my_callable and my_callable as two separate modules to reload. The first time it detects it, the rule is added to bot._callables as a string. I combined patched dgw branch 1056-no-dupes onto 1056-fix-with-recursion, to provide a more optimal solution to issue sopel-irc#1056.
HumorBaby
changed the title
|
@HumorBaby I hadn't gotten a chance to look at this yet, but was there something wrong with this PR? Mostly I'm just confused that you closed it without explanation. 😸 |
|
@dgw sorry, the CI kept failing. I worked directly off of your 1056-fix-with-recursion branch, and I think there may have been too many style issues for the Travis "checkstyle.sh" to handle. I rebased my changes onto the latest sopel master branch and am in the process of resubmitting a new PR that will hopefully not have these issues. Also, this is my first PR ever, so I am not sure I even went about this whole process the right way. I will glady take any pointers! |
|
@HumorBaby No worries, there's a first time for everything. Always glad to see new people getting involved in open source! This PR definitely could have been cleaned up later, after you worked out the CI issues. Something nobody tells you about when you're just starting is the "force push", because it can mess up other people working on the same code. It lets you clean up the history after you've made a bunch of little changes to fix stuff that wasn't building properly, and I would've been happy to help you through the process once this was all set. We'll likely go through that on #1399, once it's sorted out. I'm going to look at #1399 and see if I can figure out what's up. The checkstyle errors on this PR don't look related at first glance, so it could be interesting to see where things got messy. |
dgw
added
Bugfix
This commit attempts to address #1056 . @dgw did most of the work here, as I am working off his fork. @dgw has two branches for #1056 (one for reloading with recursion, and one for preventing duplicates). Since they both address reloading issues, I combined the changes from both branches into one. On top of that, I don't think the fix submitted in #1314 works completely for all cases. For example, when a module is defined with callables from a submodule imported into the init, additional issues arise.
For example,
my_sopel_module/some_submodule.py
my_sopel_module/__init__.py
leads to issues in the recursion, where the same module is found as
my_sopel_module.some_submodule, andsome_submodule. I added a condition to check for.in the module name, and ignore if that is the case. This should still work under all previously passing tests.