From fc27b2560dc57a5234fc16f6f1af2489b5835de6 Mon Sep 17 00:00:00 2001 From: Tim Jenness Date: Fri, 7 Jul 2023 18:02:42 -0700 Subject: [PATCH] Revert "Use importlib.resources" This reverts commit 84171147bcc000b56646246b400b6dc9b34cb3ec. The schema file API is used as a public API so we can not change it the way we did by using a context manager. --- .../alert/packet/bin/validateAvroRoundTrip.py | 24 ++++---- python/lsst/alert/packet/schema.py | 45 +++++--------- python/lsst/alert/packet/schemaRegistry.py | 20 +++---- test/test_io.py | 11 ++-- test/test_schema.py | 6 +- test/test_schemas.py | 60 +++++++++---------- 6 files changed, 72 insertions(+), 94 deletions(-) diff --git a/python/lsst/alert/packet/bin/validateAvroRoundTrip.py b/python/lsst/alert/packet/bin/validateAvroRoundTrip.py index 47032f3..51f8f41 100755 --- a/python/lsst/alert/packet/bin/validateAvroRoundTrip.py +++ b/python/lsst/alert/packet/bin/validateAvroRoundTrip.py @@ -82,18 +82,18 @@ def main(): schema_major, schema_minor = lsst.alert.packet.get_latest_schema_version() else: schema_major, schema_minor = args.schema_version.split(".") - - with lsst.alert.packet.get_schema_path(schema_major, schema_minor) as schema_root: - alert_schema = lsst.alert.packet.Schema.from_file( - os.path.join(schema_root, - schema_filename(schema_major, schema_minor)), - ) - if args.input_data: - input_data = args.input_data - else: - input_data = os.path.join(schema_root, "sample_data", SAMPLE_FILENAME) - with open(input_data) as f: - json_data = json.load(f) + schema_root = lsst.alert.packet.get_schema_path(schema_major, schema_minor) + + alert_schema = lsst.alert.packet.Schema.from_file( + os.path.join(schema_root, + schema_filename(schema_major, schema_minor)), + ) + if args.input_data: + input_data = args.input_data + else: + input_data = os.path.join(schema_root, "sample_data", SAMPLE_FILENAME) + with open(input_data) as f: + json_data = json.load(f) # Load difference stamp if included stamp_size = 0 diff --git a/python/lsst/alert/packet/schema.py b/python/lsst/alert/packet/schema.py index 5397571..85dbb27 100644 --- a/python/lsst/alert/packet/schema.py +++ b/python/lsst/alert/packet/schema.py @@ -22,11 +22,10 @@ """Routines for working with Avro schemas. """ -import contextlib import io import os.path +import pkg_resources from pathlib import PurePath -from importlib import resources import fastavro @@ -34,22 +33,10 @@ "Schema", "get_path_to_latest_schema"] -def _get_ref(*args): - """Return the package resource file path object. - - Parameters are relative to lsst.alert.packet. - """ - return resources.files("lsst.alert.packet").joinpath(*args) - - -@contextlib.contextmanager def get_schema_root(): """Return the root of the directory within which schemas are stored. - - Returned as a context manager yielding the path to the root. """ - with resources.as_file(_get_ref("schema")) as f: - yield str(f) + return pkg_resources.resource_filename(__name__, "schema") def get_latest_schema_version(): @@ -63,14 +50,12 @@ def get_latest_schema_version(): The minor version number. """ - with _get_ref("schema", "latest.txt").open("rb") as fh: - val = fh.read() + val = pkg_resources.resource_string(__name__, "schema/latest.txt") clean = val.strip() major, minor = clean.split(b".", 1) return int(major), int(minor) -@contextlib.contextmanager def get_schema_path(major, minor): """Get the path to a package resource directory housing alert schema definitions. @@ -88,11 +73,13 @@ def get_schema_path(major, minor): Path to the directory containing the schemas. """ - with resources.as_file(_get_ref("schema", str(major), str(minor))) as f: - yield str(f) + + # Note that as_posix() is right here, since pkg_resources + # always uses slash-delimited paths, even on Windows. + path = PurePath(f"schema/{major}/{minor}/") + return pkg_resources.resource_filename(__name__, path.as_posix()) -@contextlib.contextmanager def get_path_to_latest_schema(): """Get the path to the primary schema file for the latest schema. @@ -103,8 +90,8 @@ def get_path_to_latest_schema(): """ major, minor = get_latest_schema_version() - with get_schema_path(major, minor) as schema_path: - yield (PurePath(schema_path) / f"lsst.v{major}_{minor}.alert.avsc").as_posix() + schema_path = PurePath(get_schema_path(major, minor)) + return (schema_path / f"lsst.v{major}_{minor}.alert.avsc").as_posix() def resolve_schema_definition(to_resolve, seen_names=None): @@ -321,16 +308,14 @@ def from_file(cls, filename=None): if filename is None: major, minor = get_latest_schema_version() root_name = f"lsst.v{major}_{minor}.alert" - with get_schema_path(major, minor) as schema_path: - filename = os.path.join( - schema_path, - root_name + ".avsc", - ) - schema_definition = fastavro.schema.load_schema(filename) + filename = os.path.join( + get_schema_path(major, minor), + root_name + ".avsc", + ) else: root_name = PurePath(filename).stem - schema_definition = fastavro.schema.load_schema(filename) + schema_definition = fastavro.schema.load_schema(filename) if hasattr(fastavro.schema._schema, 'SCHEMA_DEFS'): # Old fastavro gives a back a list if it recursively loaded more # than one file, otherwise a dict. diff --git a/python/lsst/alert/packet/schemaRegistry.py b/python/lsst/alert/packet/schemaRegistry.py index dcfb0f4..4da2d70 100644 --- a/python/lsst/alert/packet/schemaRegistry.py +++ b/python/lsst/alert/packet/schemaRegistry.py @@ -137,15 +137,13 @@ def from_filesystem(cls, root=None, schema_root="lsst.v5_0.alert"): """ from .schema import Schema from .schema import get_schema_root - - with get_schema_root() as default_root: - if not root: - root = default_root - registry = cls() - schema_root_file = schema_root + ".avsc" - for root, dirs, files in os.walk(root, followlinks=False): - if schema_root_file in files: - schema = Schema.from_file(os.path.join(root, schema_root_file)) - version = ".".join(root.split("/")[-2:]) - registry.register_schema(schema, version) + if not root: + root = get_schema_root() + registry = cls() + schema_root_file = schema_root + ".avsc" + for root, dirs, files in os.walk(root, followlinks=False): + if schema_root_file in files: + schema = Schema.from_file(os.path.join(root, schema_root_file)) + version = ".".join(root.split("/")[-2:]) + registry.register_schema(schema, version) return registry diff --git a/test/test_io.py b/test/test_io.py index fe96f59..c0b925c 100644 --- a/test/test_io.py +++ b/test/test_io.py @@ -43,12 +43,11 @@ def setUp(self): """ self.test_schema_version = get_latest_schema_version() self.test_schema = Schema.from_file() - with get_schema_path(*self.test_schema_version) as schema_path: - sample_json_path = posixpath.join( - schema_path, "sample_data", "alert.json", - ) - with open(sample_json_path, "r") as f: - self.sample_alert = json.load(f) + sample_json_path = posixpath.join( + get_schema_path(*self.test_schema_version), "sample_data", "alert.json", + ) + with open(sample_json_path, "r") as f: + self.sample_alert = json.load(f) def _mock_alerts(self, n): """Return a list of alerts with mock values, matching diff --git a/test/test_schema.py b/test/test_schema.py index 68bef0a..c0376f4 100644 --- a/test/test_schema.py +++ b/test/test_schema.py @@ -32,8 +32,7 @@ class SchemaRootTestCase(unittest.TestCase): """ def test_get_schema_root(self): - with get_schema_root() as schema_root: - self.assertTrue(os.path.isdir(schema_root)) + self.assertTrue(os.path.isdir(get_schema_root())) class PathLatestSchemTestCase(unittest.TestCase): @@ -41,8 +40,7 @@ class PathLatestSchemTestCase(unittest.TestCase): """ def test_path_latest_schema(self): - with get_path_to_latest_schema() as schema_path: - self.assertTrue(os.path.isfile(schema_path)) + self.assertTrue(os.path.isfile(get_path_to_latest_schema())) class ResolveTestCase(unittest.TestCase): diff --git a/test/test_schemas.py b/test/test_schemas.py index 494b6d8..50d49d1 100644 --- a/test/test_schemas.py +++ b/test/test_schemas.py @@ -45,15 +45,14 @@ def test_example_json(self): no_data = ("1.0",) # No example data is available. for version in self.registry.known_versions: - with get_schema_root() as schema_root: - path = path_to_sample_data(schema_root, version, "alert.json") - schema = self.registry.get_by_version(version) # noqa: F841 - if version in no_data: - self.assertFalse(os.path.exists(path)) - else: - with open(path, "r") as f: - data = json.load(f) - self.assertTrue(self.registry.get_by_version(version).validate(data)) + path = path_to_sample_data(get_schema_root(), version, "alert.json") + schema = self.registry.get_by_version(version) # noqa: F841 + if version in no_data: + self.assertFalse(os.path.exists(path)) + else: + with open(path, "r") as f: + data = json.load(f) + self.assertTrue(self.registry.get_by_version(version).validate(data)) def test_example_avro(self): """Test that example data in Avro format can be loaded by the schema. @@ -62,28 +61,27 @@ def test_example_avro(self): bad_versions = ("2.0",) # This data is known not to parse. for version in self.registry.known_versions: - with get_schema_root() as schema_root: - path = path_to_sample_data(schema_root, version, - "fakeAlert.avro") - schema = self.registry.get_by_version(version) + path = path_to_sample_data(get_schema_root(), version, + "fakeAlert.avro") + schema = self.registry.get_by_version(version) - if version in no_data: - self.assertFalse(os.path.exists(path)) - else: - with open(path, "rb") as f: - if version in bad_versions: - with self.assertRaises(RuntimeError): - schema.retrieve_alerts(f) - else: - retrieved_schema, alerts = schema.retrieve_alerts(f) + if version in no_data: + self.assertFalse(os.path.exists(path)) + else: + with open(path, "rb") as f: + if version in bad_versions: + with self.assertRaises(RuntimeError): + schema.retrieve_alerts(f) + else: + retrieved_schema, alerts = schema.retrieve_alerts(f) - fastavro_keys = list(schema.definition.keys()) - for key in fastavro_keys: - if '__' in key and '__len__' not in key: - schema.definition.pop(key) + fastavro_keys = list(schema.definition.keys()) + for key in fastavro_keys: + if '__' in key and '__len__' not in key: + schema.definition.pop(key) - self.assertEqual(retrieved_schema, schema, - f"schema not equal on version={version}") - for idx, alert in enumerate(alerts): - self.assertTrue(schema.validate(alert), - f"failed to validate version={version}, alert idx={idx}") + self.assertEqual(retrieved_schema, schema, + f"schema not equal on version={version}") + for idx, alert in enumerate(alerts): + self.assertTrue(schema.validate(alert), + f"failed to validate version={version}, alert idx={idx}")