Fix deprecation warnings (breaking) #44
Conversation
| job = client.load_table_from_file(fileobj, | ||
| table_ref, | ||
| f"{schema}.{table}", |
There was a problem hiding this comment.
/Users/broz/src/bluelabs-joblib-python/bluelabs_joblib/db/bigquery/loader.py:50: PendingDeprecationWarning: Client.dataset is deprecated and will be removed in a future version. Use a string like 'my_project.my_dataset' or a cloud.google.bigquery.DatasetReference object, instead.
| @@ -68,7 +68,7 @@ def load(self, | |||
| session_token=aws_creds.token, manifest=True, | |||
| region=directory.loc.region, # type: ignore | |||
| empty_as_null=True, | |||
| **redshift_options) | |||
| **redshift_options) # type: ignore | |||
There was a problem hiding this comment.
I created some initial typestubs for sqlalchemy_redshift to improve our coverage here; it's not yet smart enough to understand the kwargs in detail, though.
| redshift_options['compression'] = Compression.bzip2 | ||
| else: | ||
| cant_handle_hint(fail_if_cant_handle_hint, 'compression', hints) | ||
| redshift_options['compression'] = hints['compression'] |
There was a problem hiding this comment.
sqlalchemy_redshift is now requiring that we use enums they provide. This is probably a good idea, given it looks like we were passing in the wrong strings for their BZIP2 and LZOP compression!
/Users/broz/.pyenv/versions/3.8.1/envs/bluelabs-joblib-python-3.8.1/lib/python3.8/site-packages/sqlalchemy_redshift/commands.py:429: DeprecationWarning: 'GZIP' should be, <Compression.gzip: 'GZIP'>, an instance of <enum 'Compression'>
warnings.warn(msg, DeprecationWarning)
/Users/broz/.pyenv/versions/3.8.1/envs/bluelabs-joblib-python-3.8.1/lib/python3.8/site-packages/sqlalchemy_redshift/commands.py:429: DeprecationWarning: 'UTF8' should be, <Encoding.utf8: 'UTF8'>, an instance of <enum 'Encoding'>
| :param processing_instructions: Instructions used during creation of the schema SQL. | ||
| :param records_schema: Description of the column names and types of the records. | ||
| :param include_index: If true, the Pandas dataframe index column will be included in | ||
| the move. | ||
| """ | ||
|
|
||
| warn_deprecated(schema_name=schema_name, table_name=table_name, db_engine=db_engine) |
There was a problem hiding this comment.
I missed this in the big breaking release before! This makes this a breaking PR. The next release will also include the extras breaking change, so I figured we might as well. I'll note this in the release notes and bump the middle release per semver, since we're still pre-1.0. I'll also collect the internal uses in the corresponding internal ticket and we can figure out how to handle those.
vinceatbluelabs
changed the title
| redshift_options['compression'] = Compression.bzip2 | ||
| else: | ||
| cant_handle_hint(fail_if_cant_handle_hint, 'compression', hints) | ||
| redshift_options['compression'] = hints['compression'] |
There was a problem hiding this comment.
I noticed below for encoding you have a fallback to a fixed value in the else clause, but here you pass the value through directly. Is that intentional?
There was a problem hiding this comment.
Fair enough. Come to think of it, consulting the Enum seems like a reasonable approach that would allow an old records mover to work with a new sqlalchemy-redshift, so I'll give that a shot.
|
It's one thing. |
I've been keeping a list of the deprecation errors we receive as part of Records Mover usage / builds. This takes care of a bunch of them!
This also revealed along the way that I forgot to remove some deprecated factor method arguments.
There are still some Airflow deprecation warnings that are false alarms - there's an unresolved issue here: https://issues.apache.org/jira/browse/AIRFLOW-5138