On Sat, Jul 6, 2019 at 6:15 AM Patrick Donnelly <pdonnell@xxxxxxxxxx> wrote: > > Hi Kefu, > > On Thu, Jul 4, 2019 at 5:37 AM kefu chai <tchaikov@xxxxxxxxx> wrote: > > > > On Thu, Jul 4, 2019 at 1:43 AM Patrick Donnelly <pdonnell@xxxxxxxxxx> wrote: > > > > > > Not sure who to direct this to so asking here. > > > > > > Varsha Rao has a PR [1] to add tox testing for the cephfs-shell as > > > part of the build testing. Unfortunately it seems this isn't done as > > > part of the jenkins build, probably because cephfs-shell is not an > > > install target for most platforms due to dependency issues. > > > cephfs-shell is currently only a valid target for Ubuntu 18.04+. I see > > > that jenkins is using Ubuntu for the build [2] but I'm not immediately > > > > we use whatever test slaves labeled with trusty or centos7, see > > https://github.com/ceph/ceph-build/blob/master/ceph-pull-requests/config/definitions/ceph-pull-requests.yml#L6. > > in the "make check" run you referenced, the used slave was labeled > > "jessie stretch trusty-pbuilder x86_64 bionic huge rebootable xenial > > trusty amd64". > > > > and it seems all of the trusty slaves are also labeled with xenial, > > aside from the arm64 ones. > > I'm not sure if trusty/xenial will be a problem as cephfs-shell > requires a recent version of cmd2, available on bionic. yeah, that's a valid concern, i think. but if we only perform static analyze against the code, probably we don't need to worry about the runtime dependencies like cmd2 python module. > > > > sure which flavor. Maybe enabling cephfs-shell builds is just > > > something that needs turned on the CMake flags for the jenkins build > > > (-DWITH_CEPHFS_SHELL)? > > > > i think, to test cephfs-shell with tox, in addition to > > https://github.com/ceph/ceph/pull/28239, we just need to > > > > 1. build ceph with -DWITH_PYTHON3=ON, as cephfs-shell is python3 only, > > and it uses cephfs's python binding. because both debian/control and > > ceph.spec.in require python3 development packages to be installed. so > > presumably, all trusty and centos7 slave nodes should be ready for > > building cephfs python3 binding, and for testing cephfs-shell. > > 2. add the runtime dependencies of cephfs-shell to debian/control and > > ceph.spec.in as part of the "make check" dependencies, please search > > "with make_check" in ceph.spec.in and search "# Make-Check" in > > debian/control for examples. > > > > there are two places we can add the -DWITH_PYTHON3=ON option, > > > > - https://github.com/ceph/ceph-build/blob/master/ceph-pull-requests/config/definitions/ceph-pull-requests.yml#L56 > > - https://github.com/ceph/ceph/blob/master/run-make-check.sh > > > > i'd recommend to add this option in run-make-check.sh. > > That helps a lot; thanks Kefu! I do wonder though if we need to add > the cephfs-shell dependencies. If we're currently just doing code > linting with flake8, is it really necessary? no, these dependencies are not necessary at all. i think any python version that is supported by flake8 will do. the reason the test was not triggered by jenkins was that `WITH_CEPHFS_SHELL` is not enabled by default. and neither does `run-make-check.sh` enable it . so the crux is that: shall we enable the lint test of cephfs-shell if `WITH_CEPHFS_SHELL` is not enabled. as we can always run flake8 against a program without running it or installing it. i think, probably we can make an exception anyway, even it's kinda unintuitive, to run the flake8 against cephfs-shell even `WITH_CEPHFS_SHELL` is OFF. what do you think? > > -- > Patrick Donnelly, Ph.D. > He / Him / His > Senior Software Engineer > Red Hat Sunnyvale, CA > GPG: 19F28A586F808C2402351B93C3301A3E258DD79D -- Regards Kefu Chai _______________________________________________ Dev mailing list -- dev@xxxxxxx To unsubscribe send an email to dev-leave@xxxxxxx
