fix(providers): de-register ODCS — fix the exporter-as-provider class by principle, not by name#300
Merged
Merged
Conversation
… by principle, not by name 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).
github-actions
Bot
added
provider
📄 Documentation ReminderThis PR appears to be missing a documentation reference. Our docs live in a separate repo. Please update the PR description with one of:
See the Contributing Guide for details. |
fas89
added a commit
that referenced
this pull request
Jun 27, 2026
…t robust (#304) Final-verdict polish on the provider-vs-exporter taxonomy (#300): - odps_standard/__init__ escaped auto-registration only BY ACCIDENT (it exposes two BaseProvider subclasses, so the single-subclass fallback skips it). Set __fluid_no_autoregister__ = True explicitly, matching odcs; also set it on odps/__init__ (belt-and-suspenders even though it exposes no subclass) so all three exporter packages opt out the same, robust way. - test_no_exporter_providers.py: broaden the non-deploying-apply markers to also catch a refusal via NotImplementedError / 'not implemented' (so a future exporter can't slip past with different wording), and add a consistency guard asserting every exporter package sets __fluid_no_autoregister__ — VERIFIED it fails if the flag is removed. No behaviour change: live registry still = aws/datamesh_manager/gcp/local/ redshift/snowflake (no exporter), all exporters still importable. 29 passed; ruff + black==24.10.0 clean.
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
1 participant
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.
Why
The world-class review of #298 found it was applied by name: ODPS's identical twin ODCS shipped untouched. ODCS (Open Data Contract Standard, Bitol) is a spec exporter —
OdcsProvider.apply()raises"ODCS provider does not support apply()"(odcs/provider.py:81) — yet it wasregister_provider("odcs")'d (odcs/__init__.py:29) and surfaced live influid providersas a deployment target.Verified registry-independent: every
fluid odcscommand and the ~8 importers constructOdcsProviderby direct import, so de-registration is safe.Fix the class, not the instance
providers/__init__.py—_auto_register_from_modulenow honours a module opt-out__fluid_no_autoregister__ = True, so a package that exposes aBaseProvidersubclass 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— dropregister_provider("odcs")+ set the opt-out.OdcsProviderstays importable from the package (the ~8 direct-import consumers are untouched).tests/providers/test_no_exporter_providers.py— a principled invariant: introspect every registered provider'sapply()and fail 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 confirmsdatamesh_managerandredshiftare genuine deploying providers.Verification
Live:
fluid providersno longer listsodcs(now aws/datamesh_manager/gcp/local/redshift/snowflake);fluid generate standard --format odcs+fluid odcsunaffected. 274 passed (odcs suites + provider registry);test_registry.py20 passed; ruff +black==24.10.0clean.Context
This came out of an adversarial world-class assessment whose lesson was: defects live at cross-package boundaries, and a fix-by-name leaves the sibling. The principled invariant encodes that lesson.