Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add handling for ROSE_PYTHONPATH #2736

Merged
merged 6 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@ ones in. -->

--------------------------------------------------------------------------------

## 2.2.0 (<span actions:bind='release-date'>Upcoming</span>)

### Breaking Changes

[#2736](https://github.com/metomi/rose/pull/2736) -
Rose now ignores `PYTHONPATH` to make it more robust to task environments
which set this value. If you want to add to the Rose environment itself,
e.g. to write a rose-ana test, use `ROSE_PYTHONPATH`.

## 2.1.0 (<span actions:bind='release-date'>Released 2023-07-21</span>)

### Fixes
Expand Down
36 changes: 34 additions & 2 deletions metomi/rose/rose.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,43 @@
#
# You should have received a copy of the GNU General Public License
# along with Rose. If not, see <http://www.gnu.org/licenses/>.

import os
import sys


def pythonpath_manip():
"""Stop PYTHONPATH contaminating the Rose Environment

* Remove PYTHONPATH items from sys.path to prevent PYTHONPATH
contaminating the Rose Environment.
* Add items from ROSE_PYTHONPATH to sys.path.

See Also:
https://github.com/cylc/cylc-flow/issues/5124
"""
if 'ROSE_PYTHONPATH' in os.environ:
paths = [
os.path.abspath(item)
for item in os.environ['ROSE_PYTHONPATH'].split(os.pathsep)
]
print(f"Extracted {paths} from ROSE_PYTHONPATH")
wxtim marked this conversation as resolved.
Show resolved Hide resolved
paths.extend(sys.path)
sys.path = paths

if 'PYTHONPATH' in os.environ:
for item in os.environ['PYTHONPATH'].split(os.pathsep):
abspath = os.path.abspath(item)
if abspath in sys.path:
sys.path.remove(abspath)


pythonpath_manip()


import argparse
from inspect import signature
import os
from pathlib import Path
import sys

from pkg_resources import (
DistributionNotFound,
Expand Down
44 changes: 44 additions & 0 deletions metomi/rose/tests/test_rose.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Copyright (C) British Crown (Met Office) & Contributors.
#
# This file is part of Rose, a framework for meteorological suites.
#
# Rose is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# Rose is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Rose. If not, see <http://www.gnu.org/licenses/>.
"""Test metomi/rose/rose.py
"""

import os
import sys

from metomi.rose.rose import pythonpath_manip


def test_pythonpath_manip(monkeypatch):
"""pythonpath_manip removes items in PYTHONPATH from sys.path
and adds items from ROSE_PYTHONPATH
"""
# If PYTHONPATH is set...
monkeypatch.setenv('PYTHONPATH', '/remove1:/remove2')
monkeypatch.setattr('sys.path', ['/leave-alone', '/remove1', '/remove2'])
pythonpath_manip()
# ... we don't change PYTHONPATH
assert os.environ['PYTHONPATH'] == '/remove1:/remove2'
# ... but we do remove PYTHONPATH items from sys.path, and don't remove
# items there not in PYTHONPATH
assert sys.path == ['/leave-alone']

# If ROSE_PYTHONPATH is set we retrieve its contents and
# add them to the sys.path:
monkeypatch.setenv('ROSE_PYTHONPATH', '/add1:/add2')
pythonpath_manip()
assert sys.path == ['/add1', '/add2', '/leave-alone']
2 changes: 2 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,5 @@ ignore=
E731,
# no longer best practice:
W503
; module level import not at top of file
E402,
Copy link
Member

Choose a reason for hiding this comment

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

Might be able to do this with inline comments rather than globally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was coping the Cylc tox.ini. We don't generally do this, but when we do we usually have our reasons. It's not a fault I think we'll miss.

Loading