Utilize bookmark of existing connection #196

prijendev · 2022-06-09T09:48:28Z

Description of change

To use bookmark of existing connection we have updated code as below,

Updated get_start function to use older bookmark key value if it is available for any existing connection.
- If the current bookmark_key is available in the state, then get_start returns the bookmark_key value.
- If it is not available then check and return the older_bookmark_key(if it is available) in the state for the existing connection.
- If non of the key available in the state for a particular stream, then return start_date.
Added unit test cases for the get_start function.

Note: We will not clear out existing bookmarks from the state. Instead, we will keep both bookmark values older and newer.

Manual QA steps

Run the sync mode without state and verify that the state file contains only property_hs_lastmodifieddate.
Run the sync mode with the state file(contains only hs_lastmodifieddate) and verify that the state file contains both hs_lastmodifieddate and property_hs_lastmodifieddate.
- Also, verify that tap retrieves records based on hs_lastmodifieddate.
Run the sync mode with the state file(contains both hs_lastmodifieddate and property_hs_lastmodifieddate) and verify that the state file contains both hs_lastmodifieddate and property_hs_lastmodifieddate.
- Also, verify that tap retrieves records based on property_hs_lastmodifieddate.

Risks

Rollback steps

revert this branch

karanpanchal-crest · 2022-06-10T10:44:25Z

tap_hubspot/__init__.py

+    """
+    If the current bookmark_key is available in the state, then return the bookmark_key value.
+    If it is not available then check and return the older_bookmark_key in the state for the existing connection.
+    If non of the key available in the state for a particular stream, then return start_date.


Suggested change

If non of the key available in the state for a particular stream, then return start_date.

If none of the keys are available in the state for a particular stream, then return start_date.

tap_hubspot/__init__.py

karanpanchal-crest · 2022-06-10T11:09:55Z

tap_hubspot/__init__.py

@@ -617,7 +627,14 @@ def sync_deals(STATE, ctx):
    # That's why bookmark_key is `property_hs_lastmodifieddate` so that we can mark it as automatic inclusion.

    bookmark_field_in_record = 'hs_lastmodifieddate'


Change the variable name to "last_modified_date" for readability

Updated variable name to last_modified_date

…into remove-older-replication-key-from-state

kspeer825

Will the old bookmark key be removed from state after syncing and saving the new bookmark key to state?

prijendev · 2022-07-18T10:23:47Z

Will the old bookmark key be removed from state after syncing and saving the new bookmark key to state?

No, the existing bookmark key will not be removed from the state after sync. Instead, the tap will save the old bookmark key and the new bookmark key in the state.

kspeer825 · 2022-07-20T17:56:58Z

View changes

I don't know if we should be saving an old key and value in state that will never be used. Can this just be replaced instead? Might be good to get some internal dev input here.

prijendev · 2022-07-21T12:22:31Z

View changes

I don't know if we should be saving an old key and value in state that will never be used. Can this just be replaced instead? Might be good to get some internal dev input here.

@kspeer825 I think we should keep the old bookmark key in the state for backward compatibility. We have discussed the same thing with @cosimon and concluded to keep the old bookmark key in the state in case we need to revert back for any client.

* TDL 19241 add version timestamp in contacts (#191) * Added versionTimestamp in contacts stream. * Updated bookmark test case. * Updated comments. * Updated replication method of contacts stream. * TDL 16686 Implement request timeout (#177) * Initial commit for request_timeout * Updated unit test case error * Removed path of unittest file * Updated config file * Updated readme file * Initial commit. * Updated interruptes sync offset test case * Resolved comments. * Replaced replication key of deals stream with property_hs_lastmodifieddate field. * Updated discover test case. * Updated ingterrupted_sync test case. * Utilize bookmark of existing connection (#196) * Clearing bookmark for existing connection. * Updated bookmark logic to utilize older bookmark key. * Updated logic to keep older bookmark key in the state. * Added unittest cases. * Updated unit test cases. * Resolved review comments. * Updated get_start to use old bookmark for companies stream method. * Tdl 15447 fix pylint issues (#204) * fixed pylint issues TDL-15447 * changing methods to functions since there is no-self-use * pylint issues * more pylint issues * added pylint disable comment for some scenarios * remove duplicate-code from config.yaml * pylint comment for test_bookmarks * remove duplicate code * removed wrong-import-order Co-authored-by: kethan1122 <[email protected]>

* Qa/save test artifacts (singer-io#201) * add logging and save test logs as artifacts * name should be static method so it plays well with BaseCase Co-authored-by: kspeer <[email protected]> * skip fields missing from schema for contact_lists (singer-io#202) Co-authored-by: kspeer <[email protected]> * add limitExempt to all fields test workaround (singer-io#203) Co-authored-by: kspeer <[email protected]> * TDL-19319 Make replication key as automatic inclusion (singer-io#195) * TDL 19241 add version timestamp in contacts (singer-io#191) * Added versionTimestamp in contacts stream. * Updated bookmark test case. * Updated comments. * Updated replication method of contacts stream. * TDL 16686 Implement request timeout (singer-io#177) * Initial commit for request_timeout * Updated unit test case error * Removed path of unittest file * Updated config file * Updated readme file * Initial commit. * Updated interruptes sync offset test case * Resolved comments. * Replaced replication key of deals stream with property_hs_lastmodifieddate field. * Updated discover test case. * Updated ingterrupted_sync test case. * Utilize bookmark of existing connection (singer-io#196) * Clearing bookmark for existing connection. * Updated bookmark logic to utilize older bookmark key. * Updated logic to keep older bookmark key in the state. * Added unittest cases. * Updated unit test cases. * Resolved review comments. * Updated get_start to use old bookmark for companies stream method. * Tdl 15447 fix pylint issues (singer-io#204) * fixed pylint issues TDL-15447 * changing methods to functions since there is no-self-use * pylint issues * more pylint issues * added pylint disable comment for some scenarios * remove duplicate-code from config.yaml * pylint comment for test_bookmarks * remove duplicate code * removed wrong-import-order Co-authored-by: kethan1122 <[email protected]> --------- Co-authored-by: Kyle Speer <[email protected]> Co-authored-by: kspeer <[email protected]> Co-authored-by: Prijen Khokhani <[email protected]> Co-authored-by: kethan1122 <[email protected]>

Clearing bookmark for existing connection.

26434d2

prijendev changed the title ~~Clear bookmark for existing connection.~~ Clear out bookmark for existing connections. Jun 9, 2022

prijendev added 3 commits June 9, 2022 16:49

Updated bookmark logic to utilize older bookmark key.

170abb5

Updated logic to keep older bookmark key in the state.

b251f11

Added unittest cases.

49b4d07

prijendev changed the title ~~Clear out bookmark for existing connections.~~ Utilize bookmark of existing connection Jun 10, 2022

Updated unit test cases.

ba913c5

prijendev marked this pull request as ready for review June 10, 2022 09:25

prijendev requested review from karanpanchal-crest, dbshah1212 and namrata270998 June 10, 2022 09:25

dbshah1212 approved these changes Jun 10, 2022

View reviewed changes

karanpanchal-crest suggested changes Jun 10, 2022

View reviewed changes

Resolved review comments.

fc910fd

prijendev requested review from karanpanchal-crest, cosimon and luandy64 June 10, 2022 12:33

karanpanchal-crest approved these changes Jun 10, 2022

View reviewed changes

Merge branch 'TDL-19319-make-replication-key-as-automatic-inclusion' …

e416cbf

…into remove-older-replication-key-from-state

prijendev mentioned this pull request Jun 16, 2022

TDL-19319 Make replication key as automatic inclusion #195

Merged

namrata270998 requested review from KrisPersonal and kspeer825 and removed request for karanpanchal-crest June 28, 2022 13:32

kspeer825 reviewed Jul 15, 2022

View reviewed changes

prijendev requested a review from kspeer825 July 18, 2022 10:23

kspeer825 approved these changes Jul 21, 2022

View reviewed changes

RushiT0122 approved these changes Jul 25, 2022

View reviewed changes

prijendev merged commit b7985ed into TDL-19319-make-replication-key-as-automatic-inclusion Jul 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Utilize bookmark of existing connection #196

Utilize bookmark of existing connection #196

prijendev commented Jun 9, 2022 •

edited

Loading

karanpanchal-crest Jun 10, 2022

prijendev Jun 10, 2022

karanpanchal-crest Jun 10, 2022

prijendev Jun 10, 2022

kspeer825 left a comment

prijendev commented Jul 18, 2022

kspeer825 commented Jul 20, 2022

prijendev commented Jul 21, 2022

	If non of the key available in the state for a particular stream, then return start_date.
	If none of the keys are available in the state for a particular stream, then return start_date.

		@@ -617,7 +627,14 @@ def sync_deals(STATE, ctx):
		# That's why bookmark_key is `property_hs_lastmodifieddate` so that we can mark it as automatic inclusion.

		bookmark_field_in_record = 'hs_lastmodifieddate'

Utilize bookmark of existing connection #196

Utilize bookmark of existing connection #196

Conversation

prijendev commented Jun 9, 2022 • edited Loading

Description of change

Manual QA steps

Risks

Rollback steps

karanpanchal-crest Jun 10, 2022

Choose a reason for hiding this comment

prijendev Jun 10, 2022

Choose a reason for hiding this comment

karanpanchal-crest Jun 10, 2022

Choose a reason for hiding this comment

prijendev Jun 10, 2022

Choose a reason for hiding this comment

kspeer825 left a comment

Choose a reason for hiding this comment

prijendev commented Jul 18, 2022

kspeer825 commented Jul 20, 2022

prijendev commented Jul 21, 2022

prijendev commented Jun 9, 2022 •

edited

Loading