Only set XDG_CONFIG_HOME in production web-server environment

[taldcroft]

Rationale

This bug-fix patch eliminates the issue of accidentally creating or depending on config files in $SKA/data/config.

Interface impact

No change in user interface.

Testing

This PR passes relevant unit tests in Ska and Ska3. It also passes a new test script that can only be run as a standalone script; it can't work as a pytest script (as far as I can tell) because it relies on updating the Python path prior to import kadi.

There is a new unrelated fail in test_occweb.py which is also now occurring in the installed version in Ska. This is due to some issue with the lucky configuration, but since the relevant code occweb.py is unchanged, this unit test fail is OK.

Review

This has been reviewed by Jean Connelly and John Zuhone.

Deployment

This will be installed to HEAD Ska / Ska3 flight and GRETA Ska test / Ska3 flight once approved.

This will eventually get installed to the production kadi web server (which provides the configured find_attitude functionality), but this has not been tested and must not be done without proper test, review, and FD approval. Note that the currently installed version for the web server is 3.12.2 and that is very old.

Backout

The previous version can be re-installed with no trouble if a problem arises.

cc: @jzuhone

[taldcroft]

@jzuhone - will try to get this installed in the next few days. Updating the astropy config in ska/data is not desirable.

[taldcroft]

@jeanconn - can you have a look at this and hit the approve button if you approve? I want to get Scott to review so we can install.

[jeanconn]

This seems fine to me. I'm wondering if there would be any time you'd try to run the tests from the /proj/web-kadi install (which would error on test_xdg_config_home_env_var())? For that specific case, you might save yourself a head-scratch if you wrote out something to STDERR when you modify XDG_CONFIG_HOME when running from /proj/web-kadi. Not needed.

[taldcroft]

Good comment, this got me thinking about a more transparent and better test. The new commits eliminate the need for a non-pytest test file, at the expense of needing to set PYTHONPATH by hand in the testing process. I've done this for Ska and Ska3 and tests pass (for 4 runs of pytest in total).

The instructions for updating the web server installation now include steps to run the relevant unit tests.

@jeanconn - please re-review. :smile:

[jeanconn]

I also like this idea even better; looks good. (I'll confess that I remember the quirks of the previous plan, but I don't remember the details of the non-pytest file and I think your rebase made it go away so I can't see the full evolution in the PR).

[taldcroft]

For the record here is that non-pytest test (from the emacs ~ backup). Basically it is just programmatically setting the path to include /proj/web-kadi and confirming expected behavior.

# Licensed under a 3-clause BSD style license - see LICENSE.rst
"""
Test that code near top of settings.py which optionally sets XDG_CONFIG_HOME does the
right thing of NOT setting in a non-production environment (i.e. running these tests).
"""

import os
import sys

# Tests need to be run with these unset
assert 'XDG_CONFIG_HOME' not in os.environ
assert 'XDG_CACHE_HOME' not in os.environ

# Should trigger setting XDG_CONFIG_HOME env var
sys.path.insert(0, '/proj/web-kadi')
from kadi import events

assert os.environ['XDG_CONFIG_HOME'] == os.path.join(os.environ['SKA'], 'data', 'config')
assert os.environ['XDG_CACHE_HOME'] == os.environ['XDG_CONFIG_HOME']

# Clean up
del os.environ['XDG_CONFIG_HOME']
del os.environ['XDG_CACHE_HOME']

[swolk]

OK, I need to be fully honest here, this is my first time trying to use github this way, I have often scanned the comments, and code, but not tried to delve deep. On the other hand, I have read the changes associated with this pull and they are changes to the environment setting, so I can confirm that and the testing level is appropriate. I don't know enough about the configuration to know if it will be best going forward as network configurations change. I am also practicing the comment process.

[jeanconn]

Hi @swolk, As another reviewer of this PR I note that the change to the environment setting code here has the effect that:

• for the kadi webserver -> no change
• for the kadi module when used in a process not in/on the kadi webserver -> the XDG environment variables will no longer be (incorrectly) set.

With regard to future network configurations changing; the kadi webserver should not be moving or changing. If it does, we will either have a requirement that the kadi environment still have the file path '/proj/web-kadi' in the path when run from the webserver or will reorganize that web server setup and this code.