search

rackslab / Slurm-web

Open source web dashboard for Slurm HPC clusters
https://slurm-web.com
GNU General Public License v3.0
295 stars 89 forks source link

El8 #214

Closed granquet closed 2 years ago

ellestad commented 3 years ago

Oh hey, is that a spec for a RHEL compatible rpm? I would be happy to help test a RHEL compatible version (though we're currently on CentOS 7).

granquet commented 3 years ago

Oh hey, is that a spec for a RHEL compatible rpm? I would be happy to help test a RHEL compatible version (though we're currently on CentOS 7).

yes, that's the idea. the target is red hat 8

I'm also trying to get the pythonic stuff to work with virtual envs and flask so that it's easier to deploy in test environments.

ellestad commented 3 years ago

the target is red hat 8

Yeah, we're trying to figure out what to do about the whole CentOS Stream debacle. However, licensing RHEL for every server in an HPC cluster is a bit of a dilemma for an edu organization. This week I'm working on adapting the Slurm Web Dockerfile for Singularity.

rezib commented 3 years ago

Yeah, we're trying to figure out what to do about the whole CentOS Stream debacle. However, licensing RHEL for every server in an HPC cluster is a bit of a dilemma for an edu organization. This week I'm working on adapting the Slurm Web Dockerfile for Singularity.

Hi @ellestad, current effort of @granquet for RHEL8 might work nicely on CentOS8, once ready for merge. It should also work after the complete transition of CentOS8 to Stream.

I guess some effort is still needed to natively support el7, but FYI it is planned to work on this later this year, and also RockyLinux once published.

It is awesome if you get a working singularity container for slurm-web! Feel free to open a PR for the recipe, we will be more than happy to merge it!

granquet commented 3 years ago

I propose:

setup.py
MANIFEST.in
slurmweb/
  dashboard_conf/{*.wsgi,*.py}
  restapi/{*.wsgi,*.py}
  tests/*.py

The question is (and always has been :)): how many python packages do you want to have and how "un-conventional" the setup.py can look like? (or setup.cfg) Would you want a "namespace package" ?

would you rather have 3 separate packages (that you can install separately):

import dashboard_conf
import restapi
import tests

or 3 separate packages with a namespace package added (but that you wouldn't be able to install separately):

import slurmweb # empty package
import slurmweb.dashboard_conf
import slurmweb.restapi
import slurmweb.tests

Also, please promote these great improvements by mentionning setup.py and hability to run with flask directly in the documentation! Here is probably the right spot: https://github.com/edf-hpc/slurm-web/blob/master/doc/installation.rst#from-source

I was waiting for a review on the code to start pushing some docs.

As a more cosmetic stuff, the comment of the first commit restapi: initial port to python3 has a typo (flash instead of flask) and mention unclear WIP. This might deserve an update. I've still a pending question regarding rest/auth.py that explains the WIP.

rezib commented 3 years ago

I prefer the 2nd option:

import slurmweb # empty package
import slurmweb.dashboard_conf
import slurmweb.restapi
import slurmweb.tests

I am not sure of the benefit to install separate packages using pip/setuptool as the purpose of this installation method is more for developement, tests or debugging.

With this option, we would still be able to build separate binary distribution packages (RPM or deb) for restapi and dashboard conf backend with fine grain dependencies control, then everything is OK.

granquet commented 3 years ago

With this option, we would still be able to build separate binary distribution packages (RPM or deb) for restapi and dashboard conf backend with fine grain dependencies control, then everything is OK.

so far, my answer would be: "yes and no". as the setup.py would be common to all the packages, it would mean that at least the build dependencies of all the packages are common as we are executing the setup.py as part of the construction of the packages... It doesn't change much the current status of the packages but would limit our options if we decide to fully split the packaging. As far as I understand, this is a minor hindrance we can live with.

granquet commented 3 years ago

I've updated the pythonic stuff to be in a virtual package as discussed earlier with @rezib.

I couldn't make it work exactly as discussed, I had to have an "extra" folder to represent the native virtual package.

.
├── pyproject.toml
├── setup.cfg
└── src
    └── slurmweb
        ├── confdashboard
        ├── restapi
        │   └── templates
        └── tests
            ├── mocks
            └── setups

I also had to move to a setup.cfg and use 'pip' instead of setuptools to get everything working reliably.

I need to ensure that the debian and rhel packages still generate properly (they seem to at first glance...) AND update the documentation.

rezib commented 3 years ago

Please note there is still the typo flash instead of flask in the first commit message.

Can you please explain in a few words in this commit message why setuptools does not work in our case and why we must shift to pip? What is the technical limit of setuptools in our case to justify this change? I don't have any problem with pip but I think it is important to capitalize over the issues we are facing with our deps.

I couldn't get the logic with the version numbers:

Also, the find in override_dh_install looks like a dirty solution for a misunderstood system :slightly_smiling_face: Why don't you just use debian/*.install instead?

granquet commented 3 years ago

Please note there is still the typo flash instead of flask in the first commit message.

Can you please explain in a few words in this commit message why setuptools does not work in our case and why we must shift to pip? What is the technical limit of setuptools in our case to justify this change? I don't have any problem with pip but I think it is important to capitalize over the issues we are facing with our deps.

I encountered multiple issues with the "regular" setuptools install method and the native namespace whereas the pip install method just works on every platform. Not sure why exactly, I might find some time to dig into it and have a proper explanation. I'll add something on the subject in the commit message.

I couldn't get the logic with the version numbers:

* `setup.cfg` has 1.2.3

* spec file has 3.9.6

* `debian/changelog` has 2.2.6

my bad, it's a bit of a mess:

  • setup.cfg version hasn't changed from the example I borrowed in the docs
  • and I mistook the debian/control Standards-Version for the actual software version when I created the spec file.
  • I forgot to update the debian/changelog (I guess these changes warrants a new version). I'll fix that and align everything to 2.2.7 ?

Also, the find in override_dh_install looks like a dirty solution for a misunderstood system slightly_smiling_face Why don't you just use debian/*.install instead?

Agreed, it's highly misunderstood on my side and I hacked my way into something that seemed to work :) The problem I have seems to come from the fact that '' only match files (on not folders) in the `debian/install`:

Pointers welcome.

rezib commented 3 years ago

I encountered multiple issues with the "regular" setuptools install method and the native namespace whereas the pip install method just works on every platform. Not sure why exactly, I might find some time to dig into it and have a proper explanation. I'll add something on the subject in the commit message.

:+1:

  • I forgot to update the debian/changelog (I guess these changes warrants a new version). I'll fix that and align everything to 2.2.7 ?

All these improvements deserve a bigger bump :) Let's go for 2.3.0!

  • Depending on which version of python is used to generate the package, the packages ends up installed in ./usr/lib/python3.$X/{site,dist}-packages/slurmweb_core-$VERSION

Yes, in debian/install you can use wildcards in directory names to match all versions, such as usr/lib/python*/*-packages/slurmweb_core-*.

  • There does not seem to be any way to include a folder recursively (and I found it tiresome to list all the files/folder manually)

Yes there is. If you define usr/lib/python*/slurm-web/restapi in debian/install, it will install in package all restapi directory recursively.