feat: Use SQLAlchemy core objects where possible and append only added #138
Conversation
visch
changed the title
…rget-postgres into 121-sqlalchemy-use-native
|
This ended up solving more issues than I thought it would. Added them to the main list, just wanted to call it out :) |
target_postgres/connector.py
Outdated
| new_table = sqlalchemy.Table(new_table_name, metadata, *new_columns) | ||
| new_table.create(bind=connection) | ||
| select = sqlalchemy.sql.select(table.columns) | ||
| insert = Insert(new_table).from_select(names=table.columns, select=select) |
There was a problem hiding this comment.
Major bug here, we only want to clone the metadata of the table. Might be easier to just not worry about the clone step here, and just create a temp table
…rget-postgres into 121-sqlalchemy-use-native
|
Added append only as well |
visch
changed the title
…as easier than making tests for the default public schema, if instead we just make this be set to something.
|
@visch I like where this PR is headed, leveraging sqlalchemy for as much as possible is probably a good idea. I've run into a few things while working on target-snowflake that would have benefitted from using sqlalchemy instead of building queries ourseleves. I didnt see anything worth commenting on from a quick review. I see tests aren't passing yet so feel free to @ me again once this is ready for a final full review. |
Thanks for taking a quick peak! Sorry about the failure, I missed it after I pushed this up (The tests do pass on Python 3.8+). Fixed it for Python 3.7 and added an issue here for Python 3.7 #141 As I say that another test fails :/ , fixing it now. |
Ok they are fixed now @pnadolny13 |
Closes #121, #22, #64, #119, #21, #54