[Bug] Temporal Framework Python interface does not take into account changes of mapset

[lrntct]

Describe the bug When changing the mapset inside a Python script, the temporal framework does not take this change into account and fail to connect to the temporal database, even thought GRASS reports the new mapset as being accessible.

To Reproduce

#!/usr/bin/env python3
# -*- coding: utf-8 -*-

import tempfile

import grass_session
import grass.script as gscript

session = grass_session.Session()
tmpdir = tempfile.mkdtemp()
session.open(gisdb=tmpdir,
             location='xy',
             mapset=None,
             create_opts='XY',
             loadlibs=True)

import grass.temporal as tgis

def do_something():
    tgis.init()
    tlist = gscript.read_command('t.list')
    print(tlist)

gscript.run_command('g.mapset', mapset='test1', flags='c')
print(gscript.read_command('g.mapset', flags='p'))
print(gscript.read_command('g.mapsets', flags='p'))
do_something()

gscript.run_command('g.mapset', mapset='test2', flags='c')
print(gscript.read_command('g.mapset', flags='p'))
print(gscript.read_command('g.mapsets', flags='p'))
do_something()

Output:

Loading libraries from /usr/lib/grass78/lib
Mapset switched. Your shell continues to use the history for the old mapset
You can switch the history by commands:
history -w; history -r /tmp/tmp7dc70ya4/xy/test1/.bash_history;
HISTFILE=/tmp/tmp7dc70ya4/xy/test1/.bash_history
test1

Accessible mapsets:
test1 PERMANENT

Default TGIS driver / database set to:
driver: sqlite
database: $GISDBASE/$LOCATION_NAME/$MAPSET/tgis/sqlite.db
WARNING: Temporal database connection defined as:
         /tmp/tmp7dc70ya4/xy/test1/tgis/sqlite.db
         But database file does not exist.
Creating temporal database: /tmp/tmp7dc70ya4/xy/test1/tgis/sqlite.db
----------------------------------------------

Mapset switched. Your shell continues to use the history for the old mapset
You can switch the history by commands:
history -w; history -r /tmp/tmp7dc70ya4/xy/test2/.bash_history;
HISTFILE=/tmp/tmp7dc70ya4/xy/test2/.bash_history
test2

Accessible mapsets:
test2 PERMANENT

Default TGIS driver / database set to:
driver: sqlite
database: $GISDBASE/$LOCATION_NAME/$MAPSET/tgis/sqlite.db
ERROR: Unable to execute sql statement. You have no permission to access
mapset <test2>, or mapset <test2> has no temporal database. Accessible
mapsets are: <test1>

Expected behavior I would expect the GRASS temporal framework to take into account the changes of the current mapset. This could be circumvented by the use of subprocess, but it is not convenient.

System description (please complete the following information):

• Operating System: Ubuntu 20.04
• GRASS GIS version: 7.8.2

Additional context Perhaps related: https://trac.osgeo.org/grass/ticket/2110 https://trac.osgeo.org/grass/ticket/3668

[zarch]

Hi @lrntct , I try to follow the links and the discussions that you mentioned, it is still not clear to me why running the code in a new process with subprocess fix the issue. Do you understand what is it wrong/missing?

[lrntct]

@zarch I do not know much of the internals of the temporal framework, but it seems that it sets several global variables at invocation:

https://github.com/OSGeo/grass/blob/5ae98b8f29cf011442f8a7f1cee9de0f763fda0b/lib/python/temporal/core.py#L130-L134

Maybe some of them are not properly cleaned up or reset when changing the mapset?

[wenzeslaus]

@zarch Running them in a subprocess should fix it because the init is done again.

@lrntct A reasonable workaround seems to be using the modules (through grass.script or grass.pygrass) rather than the functions. This would be more natural in GRASS context and likely more convenient than managing that through subprocesses manually.

That's not to say that the current library behavior is great, okayish perhaps.

[lrntct]

@wenzeslaus I understand that the GRASS scripting library runs each command in a subprocess. Calling modules is not very pythonic (expecting files instead of objects, etc.), and if the code makes many modules calls, it will likely be a performance hit of spawning a process each time.

[wenzeslaus]

@lrntct I agree. That's why I'm saying it's a workaround.

Just based on your note about the current implementation: The global state in the library is not very Pythonic either which is the root of the issue. Perhaps useful as a default, but some different approach is needed like an way to create a new, different object for the connection to be more flexible in what can be done.

[lrntct]

@wenzeslaus I wonder if encapsulating those functions and variables in a class could be possible, because it might help solve the issue. It would be a major refactor because those global variables are almost everywhere in core.py. It would be interesting to have the opinion of Sören on the matter, if he's still around.

[wenzeslaus]

Yes, that's a path I would try to go. Some kind of session, connection object or environment passed around to the functions (or providing the functions directly) and the functions possibly defaulting to whatever is the current/global environment (although that could be more danger than it is worth). Something similar is now possible for some functions in grass.script which take env.

[lrntct]

I'll link here the discussion about the issue in the mailing list, back in 2016.