TDL-19384 optimize logic for parent child relationship to be independent

[namrata270998]

Description of change

• Optimized the parent-child relationship to fetch the parent only a single time in case when both are selected.
• Skip emitting the parent records when it is not selected
• Execute the sync function for the parent stream when the child stream is only selected
• Use the minimum bookmark across the selected streams and sub-streams

Manual QA steps

• Verify that the parent is fetched only once when both parent and child are selected.
• Verify that parent records are not emitted when it is not selected.
• Verify that the minimum bookmark is used across the selected streams and sub-streams.

Risks

Rollback steps

• revert this branch

[karanpanchal-crest]

@namrata270998 can you please rewrite this comment as follows

Suggested change

	# We need to change the parent, when only child is selected, hence need to change
	# We need to get the parent data first for syncing the child streams. Hence, changing stream_name to parent stream when only child is selected

[namrata270998]

Updated the comment

[karanpanchal-crest]

@namrata270998 Optimization and Readability:

if is_sub_stream:
    sub_stream_name = stream_name
    stream_name = PARENT_STREAM_MAP.get(stream_name)

replication_key = STREAM_REPLICATION_KEY.get(stream_name)

You can remove the code at L578

[namrata270998]

• Updated the code
• Also We cannot remove the L578 as if both parent and child would be selected, we'll need the sub_stream_name for fetching the child records.

[karanpanchal-crest]

@namrata270998 This line overwrites L918. What is the need of this line explicitly?
If this function is called by sub-stream, we assign value to this variable at L918. Else, we keep it None as default

[namrata270998]

We should not keep it None as default, as if both parent and child would be selected, we'll need the sub_stream_name for fetching the child records.

[karanpanchal-crest]

ok. can you remove L919 as it would not have any use? It will always get overridden.

[namrata270998]

Removed the line as it will be overriden

[karanpanchal-crest]

@namrata270998 is there a reason for explicitly setting stop_window? I think we can expect that value of stop_window should be start_date + 30 days by default. This way we will check the window logic as well

[namrata270998]

Even if we keep the stop window 30 days, still the loop will execute one time, and as we just want to check that the bookmark is used properly we can keep any stop window, it will not affect anywhere in this testcase

[karanpanchal-crest]

I understand. However, it would be good if we check the routine scenarios in the test case rather than mocking it unneccesarily

[namrata270998]

Updated the stop window to take 30 days time window

[karanpanchal-crest]

Shall we not check this case in the above function itself?

[namrata270998]

In the above test case we checked for child bookmarking, and in the current we are checking for events bookmarks. Also we need to mock different things in both of them and the scenarios are different, hence kept them separate