refactor(providers): ODPS is a spec-export format, not a cloud provider#298
Merged
Conversation
ODPS (Open Data Product Standard — Bitol / LF-ODPI) is a data-product
spec/serialization format, not an infrastructure provider like
aws/gcp/snowflake/local (OdpsProvider.apply() was a no-op stub). It was
wrongly registered in the fluid_build.providers entry-point group and the
PROVIDERS registry, so it surfaced in 'fluid providers', 'fluid plugins',
and the --provider choices as if it were a deployment target.
De-register it cleanly while keeping the exporter fully working:
- Remove the odps entry-point (pyproject), the register_provider('odps'/
'opds') calls + the class import from providers/odps/__init__ (so the
package namespace exposes zero BaseProvider subclasses and the
auto-discovery scan can't re-register it), and odps from _DEFAULT_MODULES.
- Convert the one registry-coupled export path (cli/odps.py
_export_odps_v4_1) from build_provider('odps') to a direct OdpsProvider()
import, with type-only error text.
- Drop odps/opds from the top-level --provider choices.
- Guard test: tests/providers/test_odps_not_a_provider.py pins that odps/opds
are absent from the registry + entry-points + --provider, the package
exposes no BaseProvider subclass, and the exporter stays importable.
'fluid odps export' (Bitol + ODPS v4.1), 'fluid export-opds', and
'fluid generate standard --format odps' all import the exporter directly and
are unaffected. 672 passed; full ODPS export/import suites green.
…pec-exporters too Completes the ODPS reclassification: BitolOdpsProvider (odps_bitol) and OdpsStandardProvider (odps-standard) are ODPS spec-exporters in the same category as odps/opds — registered in the PROVIDERS registry but never consumed via it (verified: zero get_provider/build_provider callers; every CLI site imports them directly from fluid_build.providers.odps_standard, and they are not entry-points). Remove their register_provider() calls so no ODPS spec-exporter appears in 'fluid providers' / 'fluid plugins'. The classes stay importable from the package (the __all__ + class imports are retained for the direct-import call sites in cli/odps.py, cli/odps_standard.py, cli/generate_standard.py). The package namespace keeping two BaseProvider subclasses also means the provider auto-discovery scan's lone-subclass strategy cannot re-register them. Guard test extended to pin all four exporter names (odps/opds/odps_bitol/odps-standard) absent from the registry. Live: 'fluid providers' now shows zero odps* entries; 'fluid odps export' + 'fluid generate standard' unaffected. 586 passed.
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
… by principle, not by name (#300) 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).
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
odpswas wrongly registered as a cloud/infrastructure provider alongsideaws/gcp/snowflake/local. It isn't one: ODPS = Open Data Product Standard (Bitol / LF-ODPI) — a data-product spec / serialization format.OdpsProvider.apply()is a no-op stub ("Export action acknowledged. Use render() for actual export."), sofluid apply --provider odpsdeployed nothing.Because it was a
fluid_build.providersentry-point andregister_provider("odps"/"opds")'d in-tree, it surfaced influid providers,fluid plugins, and the--providerchoices as if it were a deployment target. Tellingly, the sibling spec format ODCS is not a provider entry-point — odps was the inconsistency.What
De-register every ODPS spec-exporter from the provider registry while keeping the exporters fully working (they're all constructed by direct import, never via the registry — verified: zero
get_provider/build_providerconsumers):pyproject.tomlodpsprovider entry-pointproviders/odps/__init__.pyregister_provider("odps"/"opds")+ the class import → package namespace exposes zeroBaseProvidersubclasses (auto-discovery scan can't re-register)providers/odps_standard/__init__.pyregister_provider("odps_bitol"/"odps-standard")(same category — Bitol exporters); classes stay importable for the direct-import call sitesproviders/__init__.pyodpsfrom_DEFAULT_MODULESpreloadcli/odps.py_export_odps_v4_1) frombuild_provider("odps")to a directOdpsProvider()import (with type-only error text) — sofluid odps export --spec odps-4.1survivescli/__init__.pyodps/opdsfrom the top-level--providerchoicesREADME.md/CHANGELOG.md--providerchangePreserved (unaffected — all direct-import)
fluid odps export(Bitol + ODPS v4.1),fluid export-opds,fluid generate standard --format odps,fluid odps import/validate,fluid odps-standard/odps-bitol.Breaking (CLI surface)
--provider odps/--provider opdsare no longer accepted — usefluid odps exportinstead (the old path was a no-op). Noted in CHANGELOG.Verification
tests/providers/test_odps_not_a_provider.pypins all four exporter names (odps/opds/odps_bitol/odps-standard) absent from the registry + entry-points +--provider, the package exposes noBaseProvidersubclass, and the exporters stay importable.ruff+black==24.10.0clean.fluid providersandfluid pluginsnow show zero odps entries (provider (4));fluid odps exportstill emits valid Bitol + v4.1 JSON;fluid generate standard --helpOK.Security
Net-positive. The only executable change (
cli/odps.py) removes an exception-text leak (str(e)→type(e).__name__, matching the type-only-logging invariant from #295–#297); everything else is removals that reduce plugin auto-load surface. No newep.load/eval/subprocesspaths.