From ed2dee2bcf8d55dc98575337da96d3b6c0a96312 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Sat, 27 Jun 2026 09:29:22 +0200 Subject: [PATCH] =?UTF-8?q?fix(providers):=20de-register=20ODCS=20?= =?UTF-8?q?=E2=80=94=20fix=20the=20exporter-as-provider=20class=20by=20pri?= =?UTF-8?q?nciple,=20not=20by=20name?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The world-class review found the ODPS fix (#298) was applied BY NAME: its identical twin ODCS shipped untouched. ODCS (Open Data Contract Standard, Bitol) is a spec exporter — OdcsProvider.apply() RAISES 'does not support apply()' — yet it was register_provider('odcs')'d and surfaced live in 'fluid providers' as a deployment target. Verified registry-independent (every 'fluid odcs' command and the ~8 importers construct it directly), so de-registration is safe. Fix the CLASS, not the instance: - providers/__init__.py: _auto_register_from_module honours a module opt-out () so a package that exposes a BaseProvider subclass for direct import but is an EXPORTER keeps the class importable without the single-subclass fallback re-registering it. Reusable for any future exporter. - providers/odcs/__init__.py: drop register_provider('odcs') + set the opt-out; OdcsProvider stays importable from the package (the ~8 direct-import consumers are untouched). - NEW tests/providers/test_no_exporter_providers.py: a PRINCIPLED invariant — introspects EVERY registered provider's apply() and fails if any is a non-deploying exporter (no-op / 'does not support apply'). This replaces the per-name check so the next sibling cannot slip; it also confirms datamesh_manager and redshift are genuine deploying providers. Live: 'fluid providers' no longer lists odcs; 'fluid generate standard --format odcs' + 'fluid odcs' unaffected. 274 passed (odcs suites + provider registry). --- fluid_build/providers/__init__.py | 12 +++- fluid_build/providers/odcs/__init__.py | 16 ++++- tests/providers/test_no_exporter_providers.py | 72 +++++++++++++++++++ 3 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 tests/providers/test_no_exporter_providers.py diff --git a/fluid_build/providers/__init__.py b/fluid_build/providers/__init__.py index 28729bb2..ef017af6 100644 --- a/fluid_build/providers/__init__.py +++ b/fluid_build/providers/__init__.py @@ -578,7 +578,17 @@ def _discover_subpackages(logger: Optional[logging.Logger]) -> None: def _auto_register_from_module(mod, logger: Optional[logging.Logger]) -> None: - """Try the three passive strategies + a single-subclass fallback.""" + """Try the three passive strategies + a single-subclass fallback. + + A module can opt OUT of all passive auto-registration by setting + ``__fluid_no_autoregister__ = True``. This is how a package that exposes a + ``BaseProvider`` subclass for direct import — but is a spec EXPORTER, not a + deployment provider (e.g. odcs) — keeps the class importable without the + single-subclass fallback silently re-registering it in the provider registry. + """ + if getattr(mod, "__fluid_no_autoregister__", False): + return + # Strategy 1: PROVIDERS dict providers_map = getattr(mod, "PROVIDERS", None) if isinstance(providers_map, dict) and providers_map: diff --git a/fluid_build/providers/odcs/__init__.py b/fluid_build/providers/odcs/__init__.py index 790d4c51..fce67207 100644 --- a/fluid_build/providers/odcs/__init__.py +++ b/fluid_build/providers/odcs/__init__.py @@ -22,9 +22,19 @@ ODCS Specification: https://github.com/bitol-io/open-data-contract-standard """ -from fluid_build.providers import register_provider - from .odcs import OdcsProvider -register_provider("odcs", OdcsProvider) +# ODCS (Open Data Contract Standard, Bitol) is a data-contract SPEC / export +# format — NOT a cloud/infrastructure provider. ``OdcsProvider.apply()`` raises +# ("does not support apply()"); it only renders/imports the ODCS spec. Like ODPS, +# it is therefore intentionally NOT registered in the provider registry and never +# appears in ``fluid providers`` / ``fluid plugins`` / ``--provider``. +# +# The class stays importable for the spec commands, which construct it directly +# (``from fluid_build.providers.odcs import OdcsProvider`` — see cli/odcs.py, +# cli/generate_standard.py, api/catalog_publication.py, datamesh_manager). The +# opt-out below stops the provider auto-discovery single-subclass fallback from +# re-registering it on the strength of that import. +__fluid_no_autoregister__ = True + __all__ = ["OdcsProvider"] diff --git a/tests/providers/test_no_exporter_providers.py b/tests/providers/test_no_exporter_providers.py new file mode 100644 index 00000000..be9b5e8f --- /dev/null +++ b/tests/providers/test_no_exporter_providers.py @@ -0,0 +1,72 @@ +# Copyright 2024-2026 Agentics Transformation Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Principle, not a name list: a *provider* deploys; an *exporter* does not. + +A registered provider whose ``apply()`` is a no-op or raises "does not support +apply" is a spec exporter masquerading as a provider (the ODPS / ODCS bug). This +introspects EVERY registered provider so the next sibling can't slip through — +replacing the per-name invariant with the underlying rule. +""" + +from __future__ import annotations + +import inspect + +from fluid_build import providers as P + +# Source markers of a non-deploying apply() — an exporter, not a provider. +_NON_DEPLOYING_MARKERS = ( + "does not support apply", + "export action acknowledged", + "use render() for actual export", +) + + +def _apply_is_non_deploying(cls) -> bool: + apply = getattr(cls, "apply", None) + if apply is None: + return False + try: + src = inspect.getsource(apply).lower() + except (OSError, TypeError): + return False + return any(marker in src for marker in _NON_DEPLOYING_MARKERS) + + +def test_no_registered_provider_is_a_non_deploying_exporter(): + P.discover_providers(force=True) + offenders = [] + for name in sorted(P.list_providers()): + cls = P.PROVIDERS.get(name) + if cls is not None and _apply_is_non_deploying(cls): + offenders.append(name) + assert not offenders, ( + "these registered providers have a non-deploying apply() — they are spec " + "EXPORTERS, not deployment providers, and must not be in the provider " + f"registry (de-register them, keep the class importable): {offenders}" + ) + + +def test_odcs_specifically_not_a_provider(): + # The instance that motivated the principle: ODCS (Open Data Contract Standard) + # is an exporter, de-registered like ODPS. + P.discover_providers(force=True) + assert "odcs" not in set(P.list_providers()) + + +def test_odcs_exporter_still_importable_directly(): + from fluid_build.providers.odcs import OdcsProvider + + assert hasattr(OdcsProvider(), "render")