Merged
Conversation
Contributor
Greptile SummaryThis PR fixes a subtle but impactful test-isolation bug: the serve test suite was directly injecting The fix replaces the
No issues were found with the new approach.
|
| Filename | Overview |
|---|---|
| test/serve/server/test_server_config.py | Removes dangerous sys.modules mocking of hydra/omegaconf (which leaked across the whole test suite) and replaces it with a proper try/except availability check + pytest.mark.skipif guard. The fix is clean — earth2studio/serve/server/config.py already guards its own hydra/omegaconf imports with try/except, so the module-level import in the test file succeeds even when the dependencies are absent. |
Last reviewed commit: "Real fix"
Collaborator
Author
|
@greptile-ai |
Collaborator
Author
|
/blossom-ci |
1 similar comment
Collaborator
Author
|
/blossom-ci |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Earth2Studio Pull Request
Description
Theres this rare issue with omegaconf when having a full install.
_____________ ERROR at setup of test_stormcast_sda_package[cuda:0] _____________ @pytest.fixture(scope="function") def sda_model() -> StormCastSDA: package = StormCastSDA.load_default_package() > return StormCastSDA.load_model(package) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ test/models/da/test_da_sda_stormcast.py:492: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ .venv/lib/python3.12/site-packages/earth2studio/utils/imports.py:177: in _wrapper return obj(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^ .venv/lib/python3.12/site-packages/earth2studio/models/da/sda_stormcast.py:396: in load_model config = OmegaConf.load(package.resolve("model.yaml")) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ .venv/lib/python3.12/site-packages/omegaconf/omegaconf.py:205: in load ret = OmegaConf.create(obj) ^^^^^^^^^^^^^^^^^^^^^ .venv/lib/python3.12/site-packages/omegaconf/omegaconf.py:178: in create return OmegaConf._create_impl( .venv/lib/python3.12/site-packages/omegaconf/omegaconf.py:900: in _create_impl format_and_raise(node=None, key=None, value=None, msg=str(e), cause=e) .venv/lib/python3.12/site-packages/omegaconf/_utils.py:819: in format_and_raise _raise(ex, cause) .venv/lib/python3.12/site-packages/omegaconf/_utils.py:797: in _raise raise ex.with_traceback(sys.exc_info()[2]) # set env var OC_CAUSE=1 for full trace ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ .venv/lib/python3.12/site-packages/omegaconf/omegaconf.py:861: in _create_impl return DictConfig( .venv/lib/python3.12/site-packages/omegaconf/dictconfig.py:111: in __init__ format_and_raise(node=None, key=key, value=None, cause=ex, msg=str(ex)) .venv/lib/python3.12/site-packages/omegaconf/_utils.py:899: in format_and_raise _raise(ex, cause) .venv/lib/python3.12/site-packages/omegaconf/_utils.py:797: in _raise raise ex.with_traceback(sys.exc_info()[2]) # set env var OC_CAUSE=1 for full trace ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ .venv/lib/python3.12/site-packages/omegaconf/dictconfig.py:109: in __init__ self._set_value(content, flags=flags) .venv/lib/python3.12/site-packages/omegaconf/dictconfig.py:647: in _set_value raise e .venv/lib/python3.12/site-packages/omegaconf/dictconfig.py:644: in _set_value self._set_value_impl(value, flags) .venv/lib/python3.12/site-packages/omegaconf/dictconfig.py:658: in _set_value_impl self._validate_set(key=None, value=value) .venv/lib/python3.12/site-packages/omegaconf/dictconfig.py:171: in _validate_set vk = get_value_kind(value) ^^^^^^^^^^^^^^^^^^^^^ .venv/lib/python3.12/site-packages/omegaconf/_utils.py:558: in get_value_kind if _is_missing_value(value): ^^^^^^^^^^^^^^^^^^^^^^^^ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ value = {'data': {'conditioning_channels': ['u10m', 'v10m', 't2m', 'tcwv', 'sp', 'msl', ...], 'conditioning_dt': 1, 'invariant...2, 2], 'channel_mult_noise': 1, ...}}, 'sampler_args': {'S_churn': 0.0, 'S_max': inf, 'S_min': 0.0, 'S_noise': 1, ...}} def _is_missing_value(value: Any) -> bool: from omegaconf import Node > if isinstance(value, Node): ^^^^^^^^^^^^^^^^^^^^^^^ E omegaconf.errors.ConfigTypeError: isinstance() arg 2 must be a type, a tuple of types, or a union E full_key: E object_type=None .venv/lib/python3.12/site-packages/omegaconf/_utils.py:511: ConfigTypeError ---------------------------- Captured stderr setup ----------------------------- Downloading config.json: 100%|██████████| 40.0/40.0 [00:00<00:00, 175B/s] Downloading model.yaml: 100%|██████████| 2.64k/2.64k [00:00<00:00, 14.0kB/s] _________________ ERROR at setup of test_aifs_package[cuda:0] __________________ @pytest.fixture(scope="function") def model() -> AIFS: # Test only on cuda device package = AIFS.load_default_package() > p = AIFS.load_model(package) ^^^^^^^^^^^^^^^^^^^^^^^^ test/models/px/test_aifs.py:815: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ .venv/lib/python3.12/site-packages/earth2studio/utils/imports.py:177: in _wrapper return obj(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^ .venv/lib/python3.12/site-packages/earth2studio/models/px/aifs.py:365: in load_model model = torch.load( .venv/lib/python3.12/site-packages/torch/serialization.py:1530: in load return _load( .venv/lib/python3.12/site-packages/torch/serialization.py:2122: in _load result = unpickler.load() ^^^^^^^^^^^^^^^^ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = <[KeyError('_content') raised in repr()] ListConfig object at 0x7c0b101c4050> d = {'_content': ['tp', 'cp', 'sf', 'ro', 'tcw', 'ssrd', ...], '_metadata': ContainerMetadata(ref_type=typing.Any, object_...'>, element_type=typing.Any), '_parent': <[KeyError('_content') raised in repr()] DictConfig object at 0x7c0b101c6ed0>} def __setstate__(self, d: Dict[str, Any]) -> None: from omegaconf import DictConfig from omegaconf._utils import is_generic_dict, is_generic_list > if isinstance(self, DictConfig): ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E TypeError: isinstance() arg 2 must be a type, a tuple of types, or a union .venv/lib/python3.12/site-packages/omegaconf/basecontainer.py:148: TypeErrorThe issue was because some mocks in the serve test suite which were marked as dangerous were infact... dangerous resulting in breaking hydra for the entire package.
This fixes the test to no mock the import...
Checklist
Dependencies