pankajastro commented on code in PR #73273:
URL: https://github.com/apache/airflow/pull/73273#discussion_r4037915959
##########
providers/common/sql/src/airflow/providers/common/sql/config.py:
##########
@@ -93,6 +96,10 @@ def __post_init__(self):
raise ValueError(f"Database name must be provided for table
providers {TABLE_PROVIDERS}")
return
+ if not self.format and not self.uri:
Review Comment:
Hoisted above the early return in 6dd30dfa20 — closes this and the
iceberg-blank-`table_name` case too.
---
Drafted-by: Claude Code (Sonnet 5); reviewed by @pankajastro before posting
##########
providers/common/sql/src/airflow/providers/common/sql/config.py:
##########
@@ -93,6 +96,10 @@ def __post_init__(self):
raise ValueError(f"Database name must be provided for table
providers {TABLE_PROVIDERS}")
return
+ if not self.format and not self.uri:
+ # Plain database table: no object store involved, so storage_type
stays unset.
Review Comment:
Moved to a follow-up: apache/airflow#73287.
---
Drafted-by: Claude Code (Sonnet 5); reviewed by @pankajastro before posting
##########
providers/common/sql/src/airflow/providers/common/sql/config.py:
##########
@@ -62,6 +62,9 @@ class DataSourceConfig:
require ``uri`` or ``storage_type``; they use ``conn_id`` and
format-specific
keys in ``options`` (e.g. ``catalog_table_name`` for Iceberg).
+ **Plain database tables** (neither ``uri`` nor ``format`` set) are not
Review Comment:
Updated in 6dd30dfa20 — `llm_schema_compare.rst` and the example DAG now
cover the plain-DB shape.
---
Drafted-by: Claude Code (Sonnet 5); reviewed by @pankajastro before posting
##########
providers/common/sql/src/airflow/providers/common/sql/config.py:
##########
@@ -93,6 +96,10 @@ def __post_init__(self):
raise ValueError(f"Database name must be provided for table
providers {TABLE_PROVIDERS}")
return
+ if not self.format and not self.uri:
+ # Plain database table: no object store involved, so storage_type
stays unset.
+ return
Review Comment:
Applied, plus one addition: also requires `uri` whenever `storage_type` is
set (closes a related gap where an explicit `storage_type` with no `uri`
slipped through). Pushed in 6dd30dfa20.
---
Drafted-by: Claude Code (Sonnet 5); reviewed by @pankajastro before posting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
