Knotinfo db interface looks for the wrong python module

[kiwifb]

Ticket #30352 included a new optional database and its interface in sage. As all new optional package of this kind, whether to use it or not is detected at runtime by the feature framework.

In #30352 presence was implemented by looking if the sage interface python module is installed. Which is always true since this interface is shipped in the standard sagelib. Instead it should have searched for the module installed by the database_knotinfo package itself.

Furthermore, it sets user writable cache under SAGE_SHARE which is a system location for any system wide installation of sage.

The problem is not clearly visible in vanilla sage as there are no direct consequence for a private user installation. But for system installation or distro installation which are sandboxed, there are consequences as sage will try to create files in system location.
See cschwan/sage-on-gentoo#639 for example.

CC: @soehms @tscrim @antonio-rojas

Component: packages: optional

Author: François Bissey

Branch: 9907095

Reviewer: Sebastian Oehms

Issue created by migration from https://trac.sagemath.org/ticket/31921

[kiwifb]

comment:2

Attach branch with patch currently tested in sage-on-gentoo. That allows documentation to be built, I am currently running doctests. The next test would be to make sure it behaves properly with the knotinfo package installed.

---

New commits:

7fe6cf9

Replace PythonModule feature by StaticFile feature.

[kiwifb]

Branch: u/fbissey/knotinfo_feature

[kiwifb]

Author: François Bissey

[kiwifb]

Commit: 7fe6cf9

[mkoeppe]

comment:3

This is not the right fix. The database is provided by a Python package so it should be looked up via import machinery.

Just change:

- PythonModule.__init__(self, 'sage.knots.knotinfo', spkg='database_knotinfo',)
+ PythonModule.__init__(self, 'database_knotinfo', spkg='database_knotinfo')

and set self._sobj_path to something like database_knotinfo.__file__

[kiwifb]

comment:4

Replying to @mkoeppe:

This is not the right fix. The database is provided by a Python package so it should be looked up via import machinery.

Just change:

- PythonModule.__init__(self, 'sage.knots.knotinfo', spkg='database_knotinfo',)
+ PythonModule.__init__(self, 'database_knotinfo', spkg='database_knotinfo')

I understand, the wrong python module was looked up. Of course you cannot know that without installing the package or reading the ticket very, very, carefully.

I will put up a corrected branch later today.

[kiwifb]

comment:5

Replying to @mkoeppe:

and set self._sobj_path to something like database_knotinfo.__file__

Python detail, do you need to import database_knotinfo before being able to query that path?

[mkoeppe]

comment:6

yes, it needs to be imported

[kiwifb]

comment:7

Hum, I have to package this to make sure I understand, but it looks to me that the current location of _sobj_path and what you suggest to do with .__file__ will be a quite different location. One should go under site-packages while the other will be under share.

[mkoeppe]

comment:8

OK, the _sobj_path is actually something else. It is a location of a cache that is created from the csv files distributed in the Python package. It's probably best to move this to a different location that is writable by users (somewhere in ~/.sage?)

[kiwifb]

comment:9

Replying to @mkoeppe:

OK, the _sobj_path is actually something else. It is a location of a cache that is created from the csv files distributed in the Python package. It's probably best to move this to a different location that is writable by users (somewhere in ~/.sage?)

Under ~/.sage should be the default for that kind of things yes. Should we put it in its own subfolder? I don't think it is necessary, as there seems to be only one object.

[soehms]

comment:11

Replying to @kiwifb:

Replying to @mkoeppe:

OK, the _sobj_path is actually something else. It is a location of a cache that is created from the csv files distributed in the Python package. It's probably best to move this to a different location that is writable by users (somewhere in ~/.sage?)

Under ~/.sage should be the default for that kind of things yes. Should we put it in its own subfolder? I don't think it is necessary, as there seems to be only one object.

There are more files, so that should be a separate folder.

Sorry for causing this trouble. I already had SAGE_DB (.sage/db) in mind instead of SAGE_SHARE when I did the last change in #30352. But since everything worked fine for me I let it as is.

I can do the tests but maybe not today!

[sagetrac-git]

Changed commit from 7fe6cf9 to 0091a4e

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

0091a4e

Use PythonModule, but this time the right one

[kiwifb]

comment:13

I am now understanding my confusion in full. All the other databases test for StaticFile and there was a file defined there in a "system location".

I'll add that since it is a user location, it doesn't really belongs to feature. Ideally, it would be defined in the package database_knotinfo itself. Let's find a location in sage for it for now, but we should keep it in mind next time the package is upgraded.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

9907095

Define _sobj_path in knotinfo_db.py itself

[sagetrac-git]

Changed commit from 0091a4e to 9907095

[kiwifb]

comment:15

Need to rewrite the description but it should be a testable state now.

[soehms]

comment:18

The patch looks good to me!

Another question: It seems that we have a new feature within our doctests, right now, which produces the following failure (independent on this branch):

sage -t --random-seed=0 src/sage/databases/knotinfo_db.py
    [90 tests, 0.06 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 0.2 seconds
    cpu time: 0.0 seconds
    cumulative wall time: 0.1 seconds
================================================================================================================================== test session starts ===================================================================================================================================
platform linux -- Python 3.9.2, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /home/sebastian/devel/sage/src, configfile: tox.ini
collected 0 items / 1 error

========================================================================================================================================= ERRORS =========================================================================================================================================
_____________________________________________________________________________________________________________________ ERROR collecting sage/databases/knotinfo_db.py _____________________________________________________________________________________________________________________
src/sage/databases/knotinfo_db.py:330: in <module>
    class KnotInfoDataBase(SageObject, UniqueRepresentation):
sage/misc/nested_class.pyx:318: in sage.misc.nested_class.NestedClassMetaclass.__init__ (build/cythonized/sage/misc/nested_class.c:2327)
    ???
E   KeyError: 'knotinfo_db'
================================================================================================================================ short test summary info =================================================================================================================================
ERROR src/sage/databases/knotinfo_db.py - KeyError: 'knotinfo_db'
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
==================================================================================================================================== 1 error in 0.73s ====================================================================================================================================

What is the reason for this? How can I fix it? Any hints are welcome!

[soehms]

Reviewer: Sebastian Oehms

[mkoeppe]

comment:19

I also see this error after installing pytest.

[mkoeppe]

comment:20

This defect is not specific to knotinfo_db; we will address it in #31924 (sage -t: Do not run pytest on individual files)

[soehms]

comment:21

Replying to @mkoeppe:

This defect is not specific to knotinfo_db; we will address it in #31924 (sage -t: Do not run pytest on individual files)

Many Thanks!

[vbraun]

Changed branch from u/fbissey/knotinfo_feature to 9907095

[soehms]

comment:23

There is another piece of code that should be adapted to the change of file-cache location. I've opened ticket #32099 for that!

[soehms]

Changed commit from 9907095 to none