cmd-ntrf / jupyter-lmod

Jupyter plugin that provides a tab for TACC Lmod (https://github.com/TACC/Lmod)
MIT License
28 stars 13 forks source link

Async function in sync decorator #63

Closed mahendrapaipuri closed 1 year ago

mahendrapaipuri commented 1 year ago

Subcommands of lmod function, which are async are wrapped in sync function. This will not work as we will be calling await on a sync function.

Example:

import lmod

await lmod.load('mymodule')
/tmp/ipykernel_2570148/1829456423.py:1: RuntimeWarning: coroutine 'API.load' was never awaited
  await lmod.load('mymodule')
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[2], line 1
----> 1 await lmod.load('mymodule')

TypeError: object NoneType can't be used in 'await' expression

We need to make print_output_decorator wrapper async as well to make it work.

def print_output_decorator(function):
    @wraps(function)
    async def wrapper(*args, **kargs):
        output = await function(*args, **kargs)
        if output is not None:
            print(output)
    return wrapper

This does not have any implication on jupyterlmod package perse as we are not using this interface for the handlers. We are essentially creating a new object LMOD out of class API and doing operations over it in handlers. However, having a working lmod.load(), lmod.unload() interface is useful for the users who wish to use them in their scripts without having to creating a new object.

cmd-ntrf commented 1 year ago

You are absolutely right. The regression was introduced in PR #58.

stebo85 commented 1 year ago

Do you have a timeline when this problem could be addressed? In our project www.Neurodesk.org we are using jupyterlmod to load module packages in python notebooks and with the latest version this broke our workflows.

cmd-ntrf commented 1 year ago

@stebo85 : thanks for the reminder. I'll implement the fix suggested by @mahendrapaipuri and release jupyter-lmod 4.0.2 today.

stebo85 commented 1 year ago

Thank you for the quick fix :)

I updated to jupyterlmod 4.0.2 and the change is in my lmod/init.py - but it's unfortunatley not working yet:

await lmod.load('module')
await lmod.list()
<coroutine object API.load at 0x7f47daf88200>
/tmp/ipykernel_260/4077590314.py:2: RuntimeWarning: coroutine 'API.load' was never awaited
  await lmod.load('module')
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

Is there anything I can do to help debugging this?

Other functions, like avail work:

await lmod.avail()
mahendrapaipuri commented 1 year ago

Ahh, @cmd-ntrf forgot to add await keyword inside the wrapper function to await the output.

@stebo85 I know its not ideal but for the time being, you can replace the print_output_decorator in the lmod/__init__.py with the following:

def print_output_decorator(function):
    @wraps(function)
    async def wrapper(*args, **kargs):
        output = await function(*args, **kargs)
        if output is not None:
            print(output)
    return wrapper
cmd-ntrf commented 1 year ago

Yep sorry about that, I finalized the fix and made the release.

stebo85 commented 1 year ago

Working perfectly now :) Thank you!