Skip to content

grass.temporal: Allow passing env - #8043

Open
saket0187 wants to merge 2 commits into
OSGeo:mainfrom
saket0187:temporal-init-env
Open

saket0187 wants to merge 2 commits into
OSGeo:mainfrom
saket0187:temporal-init-env

Conversation

@saket0187

Copy link
Copy Markdown
Contributor

Part of #7737.

tgis.init() and tgis.SQLDatabaseInterfaceConnection() now accept env, so the temporal framework can be initialized for a session which is not in os.environ, as proposed in the issue:

with gs.setup.init(path, env=os.environ.copy()) as session:
    tgis.init(env=session.env)
    dbif = tgis.SQLDatabaseInterfaceConnection(env=session.env)

With env=None, os.environ is used as before. This is the first stage from the issue: init() and the messenger and C-library subprocesses it starts. The dataset classes and the remaining grass.script calls still use os.environ and are left for follow-ups.

Details (generated with Claude Code)

Changes

  • init() reads the session (g.gisenv), runs t.connect, and reads GRASS_TGIS_RAISE_ON_ERROR and GISBASE (for the SQL templates) from env. It also uses env for its own database connection and for the database info in version messages. get_sql_template_path(), create_temporal_database() and get_database_info_string() get an env parameter for this.
  • SQLDatabaseInterfaceConnection(env=...) lists the temporal databases with t.connect in env, also in add_mapset(), through get_available_temporal_mapsets(env=...). The module-level Tools() object is replaced by Tools(env=env), because it resolved t.connect with the PATH of os.environ.
  • Messenger(env=...) and CLibrariesInterface(env=...) start their subprocess in env.
    • The subprocess replaces its os.environ before the first libgis call, because libgis reads the session on first use.
    • It also sets GISRC with G_putenv() and rereads the session file. On Windows, libgis has its own copy of the environment (see the skip reason in lib/gis/tests/lib_gis_env_test.py). With the fork start method, libgis may already have read another session in the parent.
    • env is copied to a dict because the spawn start method pickles the arguments, and os.environ cannot be pickled.

Behavior changes with env=None

  • init() creates its own Messenger instead of using the shared one from get_msgr(). On main, init() stops the shared messenger at a session change. get_msgr() then returns the same stopped object, and the next message restarts it with "WARNING: Needed to restart the messenger server". The new messenger uses the raise_on_error of the current init() call.
  • init() also restarts the subprocesses when GISRC changes, or when they were started with a different env. On main, a new session for the same mapset left the C-library server on the deleted session file of the previous session. available_mapsets() then returned [], with "Incomplete GRASS session" and a server traceback.

Verification (macOS, Python 3.14)

  • Existing tests on main and on this branch give identical results:

    • pytest for python/grass/temporal, python/grass/pygrass, python/grass/jupyter, scripts/v.what.strds and all temporal tools (407 tests);
    • the gunittest suites of grass.temporal, all temporal tools, the messenger doctests and v.what.strds with nc_spm_08 (49 files).

    The only failures, 33 in grass_pygrass_grid_test, fail on main as well.

  • Additional scripts, not added to the repo, covered:

    • init(env=...) with no session in os.environ, and with a different session there; the database is created in the mapset of env and nothing else is touched;
    • the connection with env and add_mapset();
    • the subprocesses with env;
    • the restart cases and the version messages.

    Reverting any changed line makes one of them fail. The Windows-specific line cannot be checked on macOS. The fork start method used on Linux was simulated on macOS, including a parent which had already read a session through libgis.

  • Not tested: Windows and the PostgreSQL backend.

Not covered yet

  • Some code still needs the same session in os.environ:

    • constructing a space time raster dataset (STRDSMetadata, used by open_old_stds() and tlist_grouped()) and functions called without dbif, because they create a connection without env;
    • the tool-level functions (extract, mapcalc, register, ...), because they run tools without env.

    Once these are converted, the os.environ mirroring in grass_temporal_gui_support_test.py and in datacatalog/tree.py can be removed.

  • SQLDatabaseInterfaceConnection(env=...) has to get the same environment as init(), because the current mapset and the C-library interface come from init().

  • With the PostgreSQL backend, libpq still reads the PG* variables from os.environ.

  • On Windows, only GISRC is set in the environment copy that libgis uses in the subprocesses.

The investigation and the patch were done with the assistance of Claude Code.

Claude Code was used as a coding assistant.

@github-actions github-actions Bot added Python Related code is in Python libraries labels Oct 10, 2026
Comment on lines +1203 to +1204
# On Windows, the C library has its own copy of the environment,
# and with fork, it may have read another session already.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are you meaning here? That fork is used on Windows, and does the wrong thing here?

If you relied on this assumption, we must review this PR very attentively.

You must know that the fork syscall doesn't exist at all on Windows. It's a Linux concept. On Windows you create a process, on Linux you fork an existing one, and replace info if needed (approximately that)..

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, fork isn't assumed on Windows, the comment just mixed two separate cases. Fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libraries Python Related code is in Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants