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

Generalize the extension for both Lmod and Tmod #60

Closed mahendrapaipuri closed 4 months ago

mahendrapaipuri commented 1 year ago

From my tests, the extension can be generalized to both lmod and tmod modules. The only difference is that lmod expects an env var LMOD_CMD and tmod expects MODULES_CMD. I highly doubt if there are platforms that use both of them at the same time. So, checking which env var is set should be sufficient and the rest of the logic is exactly the same.

cmd-ntrf commented 1 year ago

Interesting, I have followed tmod recent development since we switched to Lmod years ago.

Just to be certain, by tmod you mean this repo: https://github.com/cea-hpc/modules ?

If both lmod and tmod uses the same Python code generating concept, it seems easy to support both. However, it would require writing developing a few unit tests for lmod.py and a github workflow to make sure that we do not introduce changes that are incompatible with either lmod or tmod. I do some sanity check manually on our lmod system, but if we introduce support for tnod, that won't scale.

Would you be interested in contributing some tests for tmod if we move forward with this?

mahendrapaipuri commented 1 year ago

Yes, I am talking about the same project. I think folks at CEA took over this legacy tmod implementation and rebranded it like environment modules. I guess lmod and environment modules are essentially same from the functional point of view. For instance, I have environment modules installed on my local workstation and I just set a env var LMOD_CMD that points to MODULES_CMD and extension works beautifully.

Actually, I forked your project to setup this extension for our platform, which uses environment modules. I have just realised that I have written some basic tests which we can start with I guess.

cmd-ntrf commented 1 year ago

Amazing!

Thanks!

mahendrapaipuri commented 1 year ago
  • To keep things simple, do you think we can start with tmod >= 5.0? This would avoid implementing set_env.

Given that tmod 5.0 has been there around for a while (since Sep 2021), I think we can just support >= 5.0 and add a note in the documentation about this limitation in 4.*

  • The functions modify_load_gen_py_script and modify_unload_gen_py_script implements a logic that looks site specific. Do you intend to maintain your fork in the long run or should we think of a way for sites to hook in the load/unload logic of the module function and have sites implement their own logic?

I like the idea of having hooks although I am not quite sure how much it will be useful for the users. I mean it is true that I forked your project just to add that logic but honestly, I think our folks are doing it wrong. We have certain modules that are meant to "activate" a conda env. Our platform guys created the modules in a way that it emits shell commands like conda activate myenv. Obviously, for these modules, $MODULES_CMD python load does not work neither module unload. But this can be handled in more elegant way by prepending PATH with conda env bin path and native module logic would work. So, I do not know if there is actually a "need" to provide custom logic for load/unload commands.

  • Would you like to draft PRs for the test suite and the support for tmod? Or should I draft them from your fork?

I can put up a PR. So, would you like to keep the same name for the package and update the docs that the package support environment modules too?

cmd-ntrf commented 1 year ago

So, I do not know if there is actually a "need" to provide custom logic for load/unload commands.

Let's keep this as an idea for now then.

I can put up a PR. So, would you like to keep the same name for the package and update the docs that the package support environment modules too?

I would keep the same name for now and update the docs. It appears to be possible to create aliases in PyPI, so we could potentially have jupyterlmod / jupyterlmod / jupyter-tmod all points to the same package. I have to research the matter more tho.

mahendrapaipuri commented 1 year ago

I would keep the same name for now and update the docs. It appears to be possible to create aliases in PyPI, so we could potentially have jupyterlmod / jupyterlmod / jupyter-tmod all points to the same package. I have to research the matter more tho.

Fair enough. That seems like a good strategy as changing name would be a major breaking change. In that case, what do you think about reworking the code base to use a more generic name module instead of lmod? The idea is just to change the naming scheme and logic of the code base stays the same tho!

cmd-ntrf commented 1 year ago

In that case, what do you think about reworking the code base to use a more generic name module instead of lmod? The idea is just to change the naming scheme and logic of the code base stays the same tho!

Yes, you are reading my mind.

We could also implement an web handler that returns wether it's Lmod or Tmod being used, so we can adapt the logo and the mouse-over in the sidebar. Tmod logo: https://raw.githubusercontent.com/cea-hpc/modules/main/doc/img/modules_red.svg Lmod logo: https://github.com/TACC/Lmod/raw/main/logos/2x/Lmod-4color%402x.png

cmd-ntrf commented 4 months ago

Done!