Fixes #110 and #107 #111
Fixes #110 and #107 #111
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
| @@ -112,6 +112,8 @@ class GreatExpectationsOperator(BaseOperator): | |||
| :type return_json_dict: bool | |||
| :param use_open_lineage: If True (default), creates an OpenLineage action if an OpenLineage environment is found | |||
| :type use_open_lineage: bool | |||
| :param database: If provided, overwrites the default database provided by the connection | |||
There was a problem hiding this comment.
Can we not get these values from the connection extras?
I feel in general the connection should be self sufficient and exposing these parameters in the operators is separating related things(to the connection) out. Anything that is provided in the extras can override the values for database/schema, etc, no?
WDYT?
There was a problem hiding this comment.
From the docs https://airflow.apache.org/docs/apache-airflow-providers-postgres/stable/connections/postgres.html it doesn't look like database is a valid extra.
There was a problem hiding this comment.
You could get the database from self.conn.schema. The Airflow has database name in the web UI, but it's saved as 'schema` column.
There was a problem hiding this comment.
I find this whole database == schema and schema == database thing with the postgres provider to be an antipattern which is confusing for developers. One reason for this override is to clear that up. Also it may be desireable to set a different db/schema for a particular set of expectations rather than use the same for all.
| # Schema parameter always takes priority | ||
| # If a schema and db are passed as part of the data_asset_name, use that schema/db | ||
| if self.data_asset_name: | ||
| # Check if data_asset_name is in the form "SCHEMA.TABLE" or "DATABASE.TABLE.SCHEMA" |
There was a problem hiding this comment.
typo here
| # Check if data_asset_name is in the form "SCHEMA.TABLE" or "DATABASE.TABLE.SCHEMA" | |
| # Check if data_asset_name is in the form "SCHEMA.TABLE" or "DATABASE.SCHEMA.TABLE" |
|
@mpgreg Would you be interested in continuing on this PR? If not I can resume from where you started. @zhongxiao37 @pankajkoti Is there anything other than the typo and the merge conflict that needs to be fixed? |
|
@pankajkoti, I have a team that would like the same functionality. I'd like to go ahead and implement this next week if possible. Thanks! |
Addes a new attribute and operator arg for database override and parses data_asset_name for either TABLE, SCHEMA.TABLE or DATABASE.SCHEMA.TABLE.
Due to differences in how schema and database are used across database providers it may not be possible to use BaseHook to get hook and conn attributes for all variants. This PR allows redshift and postgres users to specify db and schema either as operator args or fully-qualified data_asset_name arg.