-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[MAINTENANCE] Fix schema_name deprecation warning #11849
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
base: develop
Are you sure you want to change the base?
Changes from all commits
021d17a
cbb8f59
1cb33b4
0bb5d5c
75fb9d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -502,11 +502,6 @@ filterwarnings = [ | |
| # --------------------------------------- Great Expectations Deprecation Warnings ---------------------------------- | ||
| "once:The condition_parser parameter is deprecated:DeprecationWarning", | ||
| "once:Passing a string to the row_condition parameter is deprecated:DeprecationWarning", | ||
| # Existing cloud configs store "schema_name": null. During deserialization, Pydantic passes | ||
| # None to the validator which triggers the deprecation warning. The TableAsset.dict() override | ||
| # strips schema_name from new serializations, so these will clean up over time. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like these got cleaned up from our cloud org! I think |
||
| 'once:The `schema_name` argument is deprecated:DeprecationWarning', | ||
| 'once:`schema_name` is deprecated:DeprecationWarning', | ||
|
|
||
| # --------------------------------------- TEMPORARY IGNORES -------------------------------------------------------- | ||
| # The warnings in this section should be addressed (fixed or ignored) but are ignored here temporarily to help allow | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,6 +13,8 @@ | |||||||||||
| from great_expectations.datasource.fluent import GxDatasourceWarning, SQLDatasource | ||||||||||||
| from great_expectations.datasource.fluent.sql_datasource import ( | ||||||||||||
| DEFAULT_INITIAL_QUOTE_CHARACTERS, | ||||||||||||
| MISSING, | ||||||||||||
| Missing, | ||||||||||||
| TableAsset, | ||||||||||||
| to_lower_if_not_quoted, | ||||||||||||
| ) | ||||||||||||
|
|
@@ -472,6 +474,35 @@ def test_table_name_serialization_preserves_quotes( | |||||||||||
| serialized = table_asset.dict() | ||||||||||||
| assert serialized["table_name"] == serialized_name | ||||||||||||
|
|
||||||||||||
| @pytest.mark.parametrize( | ||||||||||||
| "schema_name", | ||||||||||||
| [ | ||||||||||||
| pytest.param(MISSING, id="MISSING"), | ||||||||||||
| pytest.param(None, id="None"), | ||||||||||||
| ], | ||||||||||||
| ) | ||||||||||||
| def test_schema_name_non_string_values_normalize_to_missing_without_warning( | ||||||||||||
| self, | ||||||||||||
| schema_name: Missing | None, | ||||||||||||
| ) -> None: | ||||||||||||
| with warnings.catch_warnings(record=True) as caught: | ||||||||||||
| warnings.simplefilter("always") | ||||||||||||
| asset = TableAsset(name="my_asset", table_name="my_table", schema_name=schema_name) | ||||||||||||
|
|
||||||||||||
| deprecation_warnings = [ | ||||||||||||
| w | ||||||||||||
| for w in caught | ||||||||||||
| if issubclass(w.category, DeprecationWarning) and "schema_name" in str(w.message) | ||||||||||||
|
||||||||||||
| if issubclass(w.category, DeprecationWarning) and "schema_name" in str(w.message) | |
| if ( | |
| issubclass(w.category, DeprecationWarning) | |
| and "schema_name" in str(w.message) | |
| ) |
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.
If
v is None, that means the user passedschema_name=Noneright? We don't want that, because eventually we will removeschema_nameand break them.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.
It means that OR - and this is what Don was hitting in the ticket I believe - we hit this when loading data from the server. But I guess you're right that we probably do want to keep warning if users pass it.. maybe I should just move this to make
mercuryfilter out that fieldThere 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.
I see. It sounds like mercury "cleanup" needs to happen. Even if he was using an old asset, I wouldn't expect to see the warning.