Add Oracle Cloud Infrastructure provider and Generative AI hook - #70160
Add Oracle Cloud Infrastructure provider and Generative AI hook#70160AlejandroMorgante wants to merge 14 commits into
Conversation
0bbd204 to
7bf3b0a
Compare
AlejandroMorgante
changed the title
fcfba4b to
f97ea8c
Compare
|
@SameerMesiah97 Could you please help me with a review? Thanks! |
SameerMesiah97
left a comment
SameerMesiah97
left a comment
There was a problem hiding this comment.
Looks very clean overall. I have left some comments.
2238b37 to
8c50b5d
Compare
c96e90e to
a5d5a71
Compare
potiuk
added
the
ready for maintainer review
shahar1
left a comment
shahar1
left a comment
There was a problem hiding this comment.
Thanks for the thorough follow-up — all points from my previous review are addressed, and the security-boundary design (connection extras can't steer key_file/config_file/service_endpoint) plus the placeholder-sync test are genuinely nice. One packaging issue is blocking, one smaller bug inline.
Blocking — common-compat minimum bound too low (pyproject.toml:63)
The hooks import BaseHook, AirflowNotFoundException, and AirflowOptionalProviderFeatureException from airflow.providers.common.compat.sdk. That module only exists since apache-airflow-providers-common-compat 1.10.0, and exports these three symbols only since 1.12.0 — but the bound here stays >=1.8.0. On an environment with compat 1.8–1.11, import airflow.providers.oracle.hooks.base_oci fails with ImportError even though pip considers the environment consistent. Please bump to >=1.12.0. The # use next version marker is used when the required version isn't released yet — 1.12.0 (and 1.17.0) already are, so the marker should be dropped.
get_compartment_id() breaks for principal auth without a connection (base_oci.py:235)
self.connection.extra_dejson is read directly, so with oci_conn_id=None (instance/resource principal) this raises the misleading ValueError: An OCI connection ID is required for API key authentication., and with a missing connection row it leaks AirflowNotFoundException instead of the intended compartment error. The docs tell principal-auth users to call this method. Reusing _get_optional_connection_extras() fixes both paths — plus a small test for the no-connection case.
Smaller observations
tests/.../test_base_oci.py:38— the module-levelpytest.importorskip("oci.generative_ai")also skips the two tests whose purpose is missing-SDK behaviour (test_get_oci_sdk_requires_optional_extra,test_hook_modules_import_without_optional_oci_sdk) whenociisn't installed. Consider hosting those two in a module without the skip. Not blocking.
This review was drafted by an AI-assisted tool and confirmed by an Apache Airflow maintainer. After you've addressed the points above and pushed an update, an Apache Airflow maintainer — a real person — will take the next look at the PR. The findings cite the project's review criteria; if you think one of them is mis-applied, please reply on the PR and a maintainer will weigh in.
More on how Apache Airflow handles maintainer review: contributing-docs/05_pull_requests.rst.
Drafted-by: Claude Code (Fable 5); reviewed by @shahar1 before posting
970b3a9 to
d507fb0
Compare
shahar1
left a comment
shahar1
left a comment
There was a problem hiding this comment.
Good to go once common-compat version is settled
d507fb0 to
9435c10
Compare
|
Good point — I agree that OCI should be a separate provider, similar to microsoft.azure and microsoft.mssql. I suggest using oci as the provider ID. The alternatives oracle.cloud and oracle.oci would require restructuring the existing oracle provider, since oracle is already a provider rather than an umbrella namespace. We can still use “Oracle Cloud Infrastructure” as the public-facing name for clarity.
|
9435c10 to
52fae43
Compare
52fae43 to
ae16b25
Compare
AlejandroMorgante
changed the title
Oracle users need a shared authentication foundation for OCI services and native access to Hosted Application management APIs, matching the client-based integration pattern used by other cloud providers.
The lowest-direct-dependencies environment intentionally excludes optional FAB form packages, so Oracle tests must remain collectible without them.
Provider compatibility jobs install the base Oracle package without optional extras, so OCI-specific tests must not fail collection in those environments.
The provider integration registry only accepts operator, sensor, and transfer pages as how-to guides, so the OCI connection and service overview pages must remain regular provider documentation.
The generated API reference is spellchecked from source docstrings, so OCI response terminology must be marked as literal text to avoid a false spelling failure.
Principal-based authentication should work without an otherwise unused Airflow connection, and private signing keys must be recognized as sensitive connection data. The documentation should describe this behavior without duplicating the OCI SDK reference.
The global integration registry accepts operator, sensor, and transfer guides only. The hook-specific guide remains available from the Oracle provider documentation index without being registered as a global how-to guide.
The new OCI hooks rely on APIs newer than Oracle's recorded minimum. Without a release marker, provider release preparation cannot keep the direct dependency metadata aligned.
Principal authentication does not require an Airflow connection, and missing-SDK coverage must remain active in minimal provider environments.
The OCI hooks rely on compatibility exports introduced in common-compat 1.12.0, so older accepted installations would fail during import.
Separating cloud integrations from Oracle Database improves discoverability and lets OCI users avoid the database provider's dependencies.
Connection-managed regions must not redirect signed OCI requests, while the OCI SDK's LGPL-transitive dependency must remain an explicit optional installation.
New providers cannot appear in installable Airflow extras before their first PyPI publication, and users may enable the OCI SDK's selective service import mode.
Compatibility matrices intentionally install providers without Category X extras, so SDK-specific behavior must only be exercised when the OCI SDK is available.
da3895e to
e9010f2
Compare
|
I think I am leaning towards and possible discontinue apache-airflow-providers-oracle or handle it as an empty shell that install all oracle related packages? Migrating a provider name is something we did in the past with |
6 participants
Adds a dedicated Oracle Cloud Infrastructure (OCI) provider so OCI cloud integrations remain separate from the Oracle Database provider.
The provider uses these identifiers:
ociapache-airflow-providers-ociairflow.providers.ociScope
ociconnection type with API-key, OCI configuration-file, instance-principal, and resource-principal authentication.OciBaseHookfor OCI authentication, native client construction, connection testing, compartment resolution, and client caching.OciGenerativeAIHook, exposing the native OCIGenerativeAiClientfor Hosted Application, Hosted Deployment, and work-request operations.Security and packaging
crc32cpackage, which ASF policy classifies as Category X.pip install 'apache-airflow[oci]'forwards toapache-airflow-providers-oci[oci], so the Airflow extra installs usable OCI support whileapache-airflow[all]and the base provider avoid the Category X dependency.Testing
uv.lockare synchronized.Provider adoption
The provider adoption proposal will be sent to the Airflow dev list with the PR as its implementation reference. The required stewards, sponsoring committer, continuous validation plan, and discussion period must be confirmed before merge.
No OCI resources were created, updated, or deleted during the live smoke test.
Connection UI
This PR introduces a new Connection type, so there is no equivalent before state.
Was generative AI tooling used to co-author this PR?
Generated-by: Codex (GPT-5) following the guidelines