-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Fix Google provider to handle False boolean values in connection extras #58348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Google provider to handle False boolean values in connection extras #58348
Conversation
Fixed a bug where boolean fields set to False in Google provider connection extras were incorrectly returning default values instead. The issue occurred in two places: 1. get_field() function used 'or None' which treated False as falsy 2. _get_field() method used compound boolean logic that returned the default value when the actual value was False This particularly affected BigQueryHook where use_legacy_sql=False in connection extras would incorrectly return True. Changes: - Modified get_field() to only treat empty strings as None - Modified _get_field() to explicitly check 'is not None' - Added tests for False/falsy value handling Fixes apache#47053
c50d3b4 to
e03e41c
Compare
Update mock paths from 'airflow.hooks.base.BaseHook' to 'airflow.providers.common.compat.sdk.BaseHook' to match the new import structure in Airflow 3.x.
shahar1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests fail :(
|
First Issue(which the PR is addressing): looks like there is another issue here Scenario A: User doesn't specify anything Scenario B: User explicitly specifies a value The Problem: How do we tell the difference?
The challenge: We need a way to say "this parameter wasn't provided at all" vs "this parameter was provided with value False/True/None" |
Use sentinel pattern to distinguish unset parameters from explicitly False values, allowing both connection extras and parameter overrides to work correctly.
…as (apache#58348) * Fix Google provider to handle False boolean values in connection extras Fixed a bug where boolean fields set to False in Google provider connection extras were incorrectly returning default values instead. The issue occurred in two places: 1. get_field() function used 'or None' which treated False as falsy 2. _get_field() method used compound boolean logic that returned the default value when the actual value was False This particularly affected BigQueryHook where use_legacy_sql=False in connection extras would incorrectly return True. Changes: - Modified get_field() to only treat empty strings as None - Modified _get_field() to explicitly check 'is not None' - Added tests for False/falsy value handling Fixes apache#47053 * Fix test import paths for Airflow 3.x compatibility Update mock paths from 'airflow.hooks.base.BaseHook' to 'airflow.providers.common.compat.sdk.BaseHook' to match the new import structure in Airflow 3.x. * Fix Google provider to respect False boolean values in connection extras Use sentinel pattern to distinguish unset parameters from explicitly False values, allowing both connection extras and parameter overrides to work correctly. * Fix mypy no-redef errors by removing duplicate type annotations * Fix import order for static checks * Trigger CI re-run
…as (apache#58348) * Fix Google provider to handle False boolean values in connection extras Fixed a bug where boolean fields set to False in Google provider connection extras were incorrectly returning default values instead. The issue occurred in two places: 1. get_field() function used 'or None' which treated False as falsy 2. _get_field() method used compound boolean logic that returned the default value when the actual value was False This particularly affected BigQueryHook where use_legacy_sql=False in connection extras would incorrectly return True. Changes: - Modified get_field() to only treat empty strings as None - Modified _get_field() to explicitly check 'is not None' - Added tests for False/falsy value handling Fixes apache#47053 * Fix test import paths for Airflow 3.x compatibility Update mock paths from 'airflow.hooks.base.BaseHook' to 'airflow.providers.common.compat.sdk.BaseHook' to match the new import structure in Airflow 3.x. * Fix Google provider to respect False boolean values in connection extras Use sentinel pattern to distinguish unset parameters from explicitly False values, allowing both connection extras and parameter overrides to work correctly. * Fix mypy no-redef errors by removing duplicate type annotations * Fix import order for static checks * Trigger CI re-run
…as (apache#58348) * Fix Google provider to handle False boolean values in connection extras Fixed a bug where boolean fields set to False in Google provider connection extras were incorrectly returning default values instead. The issue occurred in two places: 1. get_field() function used 'or None' which treated False as falsy 2. _get_field() method used compound boolean logic that returned the default value when the actual value was False This particularly affected BigQueryHook where use_legacy_sql=False in connection extras would incorrectly return True. Changes: - Modified get_field() to only treat empty strings as None - Modified _get_field() to explicitly check 'is not None' - Added tests for False/falsy value handling Fixes apache#47053 * Fix test import paths for Airflow 3.x compatibility Update mock paths from 'airflow.hooks.base.BaseHook' to 'airflow.providers.common.compat.sdk.BaseHook' to match the new import structure in Airflow 3.x. * Fix Google provider to respect False boolean values in connection extras Use sentinel pattern to distinguish unset parameters from explicitly False values, allowing both connection extras and parameter overrides to work correctly. * Fix mypy no-redef errors by removing duplicate type annotations * Fix import order for static checks * Trigger CI re-run
Fixes #47053
Problem
Boolean fields set to
Falsein Google provider connection extras incorrectly return default values. This affectsBigQueryHookwhereuse_legacy_sql=FalsereturnsTrueinstead.The bug is in
base_google.py:get_field(): Usesor Nonewhich treatsFalseas falsy_get_field(): Usesand ... or defaultlogic that returns default when value isFalseRoot Cause
Code used or None pattern which treats False as falsy. Additionally, couldn't distinguish "parameter not provided" from "parameter explicitly False".
Solution
Implemented sentinel pattern with _UNSET = object() to distinguish unset parameters from explicitly False values.
Changes