-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat: Activate Version with example test #89
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
e25b99d
Activat Version example test
visch 3280b01
Activate version working
visch 43dc675
Lint and Schema fix for config
visch c433682
Activate Version test to show data being deleted
visch b367081
Lint Fix
visch f908796
Merge branch 'main' into activate_version
visch 2ba215b
Activate version is doing things correctly, existing implementations …
visch fff8458
Merge branch 'activate_version' of github.com:MeltanoLabs/target-post…
visch 67989b3
Merge branch 'main' into activate_version
visch e3fad47
Merge branch 'main' into activate_version
visch 42441da
Merge branch 'main' into activate_version
visch f40ba63
Consistent spacing for docs
visch aacbed1
Update README.md
visch File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,12 @@ | |
import uuid | ||
from typing import Any, Dict, Iterable, List, Optional, Union | ||
|
||
import sqlalchemy | ||
from pendulum import now | ||
from singer_sdk.sinks import SQLSink | ||
from sqlalchemy import Column, MetaData, Table, insert | ||
from sqlalchemy.sql import Executable | ||
from sqlalchemy.sql.expression import bindparam | ||
|
||
from target_postgres.connector import PostgresConnector | ||
|
||
|
@@ -252,3 +255,67 @@ def schema_name(self) -> Optional[str]: | |
|
||
# Schema name not detected. | ||
return None | ||
|
||
def activate_version(self, new_version: int) -> None: | ||
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. I think we could take this function and just drop it into the SDK as it is here, this just has a few tweaks that are needed to make this work in a more generic way |
||
"""Bump the active version of the target table. | ||
|
||
Args: | ||
new_version: The version number to activate. | ||
""" | ||
# There's nothing to do if the table doesn't exist yet | ||
# (which it won't the first time the stream is processed) | ||
if not self.connector.table_exists(self.full_table_name): | ||
return | ||
|
||
deleted_at = now() | ||
# Different from SingerSDK as we need to handle types the | ||
# same as SCHEMA messsages | ||
datetime_type = self.connector.to_sql_type( | ||
{"type": "string", "format": "date-time"} | ||
) | ||
|
||
# Different from SingerSDK as we need to handle types the | ||
# same as SCHEMA messsages | ||
integer_type = self.connector.to_sql_type({"type": "integer"}) | ||
|
||
if not self.connector.column_exists( | ||
full_table_name=self.full_table_name, | ||
column_name=self.version_column_name, | ||
): | ||
self.connector.prepare_column( | ||
self.full_table_name, | ||
self.version_column_name, | ||
sql_type=integer_type, | ||
) | ||
|
||
self.logger.info("Hard delete: %s", self.config.get("hard_delete")) | ||
if self.config["hard_delete"] is True: | ||
self.connection.execute( | ||
f"DELETE FROM {self.full_table_name} " | ||
visch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
f"WHERE {self.version_column_name} <= {new_version} " | ||
f"OR {self.version_column_name} IS NULL" | ||
) | ||
return | ||
|
||
if not self.connector.column_exists( | ||
full_table_name=self.full_table_name, | ||
column_name=self.soft_delete_column_name, | ||
): | ||
self.connector.prepare_column( | ||
self.full_table_name, | ||
self.soft_delete_column_name, | ||
sql_type=datetime_type, | ||
) | ||
# Need to deal with the case where data doesn't exist for the version column | ||
query = sqlalchemy.text( | ||
f"UPDATE {self.full_table_name}\n" | ||
f"SET {self.soft_delete_column_name} = :deletedate \n" | ||
f"WHERE {self.version_column_name} < :version " | ||
f"OR {self.version_column_name} IS NULL \n" | ||
f" AND {self.soft_delete_column_name} IS NULL\n" | ||
) | ||
query = query.bindparams( | ||
bindparam("deletedate", value=deleted_at, type_=datetime_type), | ||
bindparam("version", value=new_version, type_=integer_type), | ||
) | ||
self.connector.connection.execute(query) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
10 changes: 10 additions & 0 deletions
10
target_postgres/tests/data_files/activate_version_hard.singer
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{"type": "SCHEMA", "stream": "test_activate_version_hard", "schema": {"type": "object", "properties": {"code": {"type": ["string"]}, "name": {"type": ["null", "string"]}}}, "key_properties": ["code"], "bookmark_properties": []} | ||
{"type": "ACTIVATE_VERSION", "stream": "test_activate_version_hard", "version": 1674486431563} | ||
{"type": "RECORD", "stream": "test_activate_version_hard", "record": {"code": "AF", "name": "Africa"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_hard", "record": {"code": "AN", "name": "Antarctica"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_hard", "record": {"code": "AS", "name": "Asia"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_hard", "record": {"code": "EU", "name": "Europe"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_hard", "record": {"code": "NA", "name": "North America"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_hard", "record": {"code": "OC", "name": "Oceania"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_hard", "record": {"code": "SA", "name": "South America"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "ACTIVATE_VERSION", "stream": "test_activate_version_hard", "version": 1674486431563} |
10 changes: 10 additions & 0 deletions
10
target_postgres/tests/data_files/activate_version_soft.singer
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{"type": "SCHEMA", "stream": "test_activate_version_soft", "schema": {"type": "object", "properties": {"code": {"type": ["string"]}, "name": {"type": ["null", "string"]}}}, "key_properties": ["code"], "bookmark_properties": []} | ||
{"type": "ACTIVATE_VERSION", "stream": "test_activate_version_soft", "version": 1674486431563} | ||
{"type": "RECORD", "stream": "test_activate_version_soft", "record": {"code": "AF", "name": "Africa"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_soft", "record": {"code": "AN", "name": "Antarctica"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_soft", "record": {"code": "AS", "name": "Asia"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_soft", "record": {"code": "EU", "name": "Europe"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_soft", "record": {"code": "NA", "name": "North America"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_soft", "record": {"code": "OC", "name": "Oceania"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_soft", "record": {"code": "SA", "name": "South America"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "ACTIVATE_VERSION", "stream": "test_activate_version_soft", "version": 1674486431563} |
10 changes: 10 additions & 0 deletions
10
target_postgres/tests/data_files/test_activate_version_deletes_data_properly.singer
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{"type": "SCHEMA", "stream": "test_activate_version_deletes_data_properly", "schema": {"type": "object", "properties": {"code": {"type": ["string"]}, "name": {"type": ["null", "string"]}}}, "key_properties": ["code"], "bookmark_properties": []} | ||
{"type": "ACTIVATE_VERSION", "stream": "test_activate_version_deletes_data_properly", "version": 1674486431563} | ||
{"type": "RECORD", "stream": "test_activate_version_deletes_data_properly", "record": {"code": "AF", "name": "Africa"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_deletes_data_properly", "record": {"code": "AN", "name": "Antarctica"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_deletes_data_properly", "record": {"code": "AS", "name": "Asia"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_deletes_data_properly", "record": {"code": "EU", "name": "Europe"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_deletes_data_properly", "record": {"code": "NA", "name": "North America"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_deletes_data_properly", "record": {"code": "OC", "name": "Oceania"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "RECORD", "stream": "test_activate_version_deletes_data_properly", "record": {"code": "SA", "name": "South America"}, "version": 1674486431563, "time_extracted": "2023-01-23T15:07:11.563063Z"} | ||
{"type": "ACTIVATE_VERSION", "stream": "test_activate_version_deletes_data_properly", "version": 1674486431563} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Boolean should be generated as True / False here I think? I'll leave for now
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.
Agreed. Very strange that it shows defaults of
0
. Agreed though we don't need to block on this.