DeanLight / spannerlib

https://deanlight.github.io/spannerlib/
Apache License 2.0
3 stars 1 forks source link

nbdev_test having issues because of Python caching modules #118

Closed loayshaqir1 closed 12 months ago

loayshaqir1 commented 1 year ago

Ill explain the issue briefly again: nbdev_test uses 8 workers (threads) to run the tests, because we have more than 8 files to test then there will be workers that will run more than one test, for each test running we would've wanted that the worker start from scratch and not have rgxlog in sys.modules already. unfortunately, that's not the case as nbdev don't clear anything from a worker before moving him to run another test, and due to that we may get conflicts on the magic_session as it is a global variable in init file.

The solution that we suggested yesterday to put this inside the init.py file:

if __name__ == 'rgxlog':
    magic_session = Session()

did not work and that is because of the Python caching modules, which means that the worker that already has rgxlog in sys.modules won't even get into init.py again. this also means that there is nothing that we can do from inside the init.py that could help us.

The solution @DeanLight suggested was to make each user initiate magic_session on his own plus passing that to the IPython magic to run cells in that session. This solution a little bit affects the user experience so i suggest that we wait on it until we try and explore new options.

The solution that i have on my mind right now: If I think about that, its completely nbdev's issue that they don't clear workers when moving them from test to test and its not our users that should pay the price, so we can have a python file that looks like that:

import subprocess
from nbdev.doclinks import nbglob

files_to_test = nbglob()
for file in files_to_test:
    command = f"nbdev_test --path {file} --do_print"
    try:
        subprocess.run(command, shell=True, check=True)
    except subprocess.CalledProcessError as e:
        print(f"Error occurred while testing {file}: {e}")

that file will call nbdev_test separately on each test file and that makes sure that each file gets a fresh new worker to test him, and instead of using nbdev_test in the terminal or ci, we can simply replace that by python nbdev_test.py (Until we open an issue in nbdev and it gets solved from their end)

What do you think about that?

DeanLight commented 1 year ago

Sure, lets go with that.