Skip to content

Commit

Permalink
[runtime env] Close schema after loading and continue on error (ray-p…
Browse files Browse the repository at this point in the history
…roject#33535)

This PR fixes a few things:

* A warning from not closing the file opened with `open()`. (We have these warnings as errors and Ray was causing some integration tests to blink)
* Using a custom runtime env schema with `RAY_RUNTIME_ENV_PLUGIN_SCHEMAS` would result in a failure when the JSON file is incorrectly decoded or the file doesn't exist.
    * There was a test for invalid decoded JSON, but by chance it ran *after* a previous schema, meaning the missing `continue` wasn't noticed.

**Steps to Reproduce**
1. Save this script as `test.py`
```python
import ray

@ray.remote(runtime_env={"env_vars": {}})
def my_fn():
    return True

ray.init()
print(ray.get(my_fn.remote()))
```
2. run with `RAY_RUNTIME_ENV_PLUGIN_SCHEMAS=./non-exist.json python test.py`
3.
    a. save `:` or other invalid JSON as `bad-json.json`
    b. run with `RAY_RUNTIME_ENV_PLUGIN_SCHEMAS=./bad-json.json python test.py`

This PR fixes the issue and adds a new test case.
Signed-off-by: James Clark <[email protected]>
Signed-off-by: elliottower <[email protected]>
  • Loading branch information
jamesclark-Zapata authored and elliottower committed Apr 22, 2023
1 parent 1cb46bf commit 85abbda
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
7 changes: 6 additions & 1 deletion python/ray/_private/runtime_env/plugin_schema_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,14 @@ class RuntimeEnvPluginSchemaManager:
def _load_schemas(cls, schema_paths: List[str]):
for schema_path in schema_paths:
try:
schema = json.load(open(schema_path))
with open(schema_path) as f:
schema = json.load(f)
except json.decoder.JSONDecodeError:
logger.error("Invalid runtime env schema %s, skip it.", schema_path)
continue
except OSError:
logger.error("Cannot open runtime env schema %s, skip it.", schema_path)
continue
if "title" not in schema:
logger.error(
"No valid title in runtime env schema %s, skip it.", schema_path
Expand Down
15 changes: 14 additions & 1 deletion python/ray/tests/test_runtime_env_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,11 +303,24 @@ def test_parse_runtime_env_from_json_env_variable(self):
test_env_2 = os.path.join(
os.path.dirname(__file__), "test_runtime_env_validation_2_schema.json"
)
test_env_invalid_path = os.path.join(
os.path.dirname(__file__), "test_runtime_env_validation_non_existent.json"
)
test_env_bad_json = os.path.join(
os.path.dirname(__file__), "test_runtime_env_validation_bad_2_schema.json"
)


@pytest.mark.parametrize(
"set_runtime_env_plugin_schemas",
[schemas_dir, f"{test_env_1},{test_env_2}"],
[
schemas_dir,
f"{test_env_1},{test_env_2}",
# Test with an invalid JSON file first in the list
f"{test_env_bad_json},{test_env_1},{test_env_2}",
# Test with a non-existent JSON file
f"{test_env_invalid_path},{test_env_1},{test_env_2}",
],
indirect=True,
)
@pytest.mark.skipif(sys.platform == "win32", reason="Failing on Windows.")
Expand Down

0 comments on commit 85abbda

Please sign in to comment.