Skip to content
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

TDL-19957 Update dict based to class based implementation #64

Conversation

namrata270998
Copy link
Contributor

@namrata270998 namrata270998 commented Jul 22, 2022

Description of change

TDL 19957 update dict based implementation to class based

  • Changed dict-based implementation to class-based implementation.
  • Created functionality-wise separate files - schema, discover, sync, streams.
  • Moved logic of REST API call and error handling to client.py.
  • Created class for incremental, full_table sync.
  • Created parent class and class for each stream.
  • Added details code comments for each function.
  • Added detailed unit test case.
  • Removed unnecessary unit test case.
  • Added implementation to write separate bookmark for child and parent stream.
  • Tap use a minimum bookmark among the selected parent and its child stream.
  • Tap write maximum bookmark for all selected parents and its child's stream
  • Tap write all child records of newly updated parent records.
  • Tap write replication key value of parent's record in the state of child stream.
  • Split landings into submitted_landings and unsubmitted_landings and introduced a new replication key for both.

Manual QA steps

Risks

Rollback steps

  • revert this branch

@namrata270998 namrata270998 changed the base branch from crest-master to TDL-19964-add-missing-tap-tester-tests July 27, 2022 05:41
@namrata270998 namrata270998 changed the title Tdl 19957 Update dict based to class based implementation TDK 19957 Update dict based to class based implementation Jul 28, 2022
@namrata270998 namrata270998 changed the title TDK 19957 Update dict based to class based implementation TDL 19957 Update dict based to class based implementation Jul 28, 2022
@namrata270998 namrata270998 changed the title TDL 19957 Update dict based to class based implementation TDL-19957 Update dict based to class based implementation Jul 28, 2022

mismatched_forms = _compare_forms(config_forms, api_forms)
mismatched_forms = config_forms.difference(api_forms)

if len(mismatched_forms) > 0:
LOGGER.fatal(f"FormMistmatchError: forms {mismatched_forms} not returned by API")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add an form mismatch message in the FormMistmatchError exception raised below instead of defining Logger for the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the message

key_properties = ['form_id', 'question_id']
endpoint = 'forms/{}'
params = {
'since': '',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please remove this 'since' param? As it isn't used for the FULL_TABLE stream.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the param

def add_fields_at_1st_level(self, record, additional_data = {}):
data_type = record.get('type')

if data_type in ['choice', 'choices', 'payment']:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add code comments

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comments

else:
answer_value = record.get(data_type)

record.update({

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add code comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comments

return Mocksession(json_data, status_code, content, headers, raise_error)

from tap_typeform.client import ERROR_CODE_EXCEPTION_MAPPING
import tap_typeform.client as client_

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unit tests for the TypeformError for method raise_for_error are missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added unittests

self.assertEqual(mocked_session.call_count, 3)

with self.assertRaises(error):
client.request(url)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unittests of raw data items Logger is missing for the request method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added unittests

class TestAddFieldAt1StLevel(unittest.TestCase):
"""
Test `add_fields_at_1st_level` method for all streams.
"""

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Below lines aren't covered in test cases:

answer_value = json.dumps(record.get(data_type))

answer_value = str(record.get(data_type))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added unittests

try:
if page_size is None:
pass
elif int(float(page_size)) > 0:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment explaining the int(float()). It's not immediately clear why there is a conversion to float before int.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are typecasting first to float to handle the corner scenarios such as string float values i.e. "10.6" which would throw an exception when directly typecasted to int

@prijendev prijendev merged commit 1c636fd into TDL-19964-add-missing-tap-tester-tests Aug 29, 2022
prijendev pushed a commit that referenced this pull request Aug 29, 2022
* TDL-19964 Added missing tap-tester commits

* updated tap-tester tests

* updated bookmarks tap-tester test

* Updated and removed extra comments

* updated start date as per format

* updated pagination test case

* resolved review comments

* added new stream in tap-tester

* fixed the cci issues

* added missing assertion for all fields

* removed get_logger()

* added logger instead of print

* TDL-19957 Update dict based to class based implementation (#64)

* TDL-19957 update dict based to class based

* updated while condition

* updated while condition

* removed incremental_range from REQUIRED_CONFIG_KEYS

* updated discover and schema file

* updated to replication_key instead of keys

* updated schemas and added comments

* added unittests for code coverage

* added unittests for sync.py

* added parameterized in setup.py

* added parameterized in config.yml and updated unittests

* updated the replication key to list instead of a single key

* fixed the issue for Keyerror of form_id in answers stream

* updated unittests

* added configurable page size param

* handled page_size for 0 and updated unittests

* resolved bug fixes for pagination and removed incompleted_forms_only param

* added new stream unsubmitted landings

* resolved PR review comments

* added page_size in example config

* added back incremental_range in the tap-tester

* raised exc instead of fatal error message and updated unittests

* resolved PR comments

* fixed the issue when page_size not passed in config
@prijendev prijendev mentioned this pull request Sep 2, 2022
somethingmorerelevant pushed a commit that referenced this pull request Sep 20, 2022
* Tdl 19964 add missing tap tester tests (#65)

* TDL-19964 Added missing tap-tester commits

* updated tap-tester tests

* updated bookmarks tap-tester test

* Updated and removed extra comments

* updated start date as per format

* updated pagination test case

* resolved review comments

* added new stream in tap-tester

* fixed the cci issues

* added missing assertion for all fields

* removed get_logger()

* added logger instead of print

* TDL-19957 Update dict based to class based implementation (#64)

* TDL-19957 update dict based to class based

* updated while condition

* updated while condition

* removed incremental_range from REQUIRED_CONFIG_KEYS

* updated discover and schema file

* updated to replication_key instead of keys

* updated schemas and added comments

* added unittests for code coverage

* added unittests for sync.py

* added parameterized in setup.py

* added parameterized in config.yml and updated unittests

* updated the replication key to list instead of a single key

* fixed the issue for Keyerror of form_id in answers stream

* updated unittests

* added configurable page size param

* handled page_size for 0 and updated unittests

* resolved bug fixes for pagination and removed incompleted_forms_only param

* added new stream unsubmitted landings

* resolved PR review comments

* added page_size in example config

* added back incremental_range in the tap-tester

* raised exc instead of fatal error message and updated unittests

* resolved PR comments

* fixed the issue when page_size not passed in config

* TDL-19801: Tap fetch data for sub-questions (#62)

* TDL-19801: Tap does not support fetching data for the questions nested within a Question Group.

* addressed the comments

* add unittest

* modify funciton comment

* formated test_case value in unittest

* formated expected_case

* Updated schema in questions.json

* resolved build fail error

* resolve build fail error

* updated unittest

* updated setup.py

* add parameterized

* change start date in start_date_test

* make change in bookmark test

* Updated unit test case.

* Updated schemas and keyerror.

Co-authored-by: Jay Tilala <jay.tilala@CDSYS.LOCAL>
Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com>

* TDL-19959 added missing fields (#63)

* added missing fields

* TDL-19957 update dict based to class based

* updated while condition

* updated while condition

* removed incremental_range from REQUIRED_CONFIG_KEYS

* TDL-19964 Added missing tap-tester commits

* updated discover and schema file

* updated tap-tester tests

* updated to replication_key instead of keys

* updated schemas and added comments

* added unittests for code coverage

* updated bookmarks tap-tester test

* added unittests for sync.py

* added parameterized in setup.py

* added parameterized in config.yml and updated unittests

* updated the replication key to list instead of a single key

* Updated and removed extra comments

* fixed the issue for Keyerror of form_id in answers stream

* updated unittests

* updated start date as per format

* added configurable page size param

* updated pagination test case

* handled page_size for 0 and updated unittests

* resolved review comments

* resolved bug fixes for pagination and removed incompleted_forms_only param

* updated array type schema

* added new stream unsubmitted landings

* added new stream in tap-tester

* updated indentation

* resolved PR review comments

* updated indentation to use 2 spaces

* added page_size in example config

* added back incremental_range in the tap-tester

* fixed the cci issues

* added missing fields to a dict

* added missing assertion for all fields

* updated comment

* raised exc instead of fatal error message and updated unittests

* removed get_logger()

* added logger instead of print

* Updated schema for questions.

* Removed duplicate assertion in all_fields test.

Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com>

* Updated schema for answers stream.

Co-authored-by: namrata270998 <75604662+namrata270998@users.noreply.github.com>
Co-authored-by: jtilala <104966482+jtilala@users.noreply.github.com>
Co-authored-by: Jay Tilala <jay.tilala@CDSYS.LOCAL>
nitingaikwad1 pushed a commit that referenced this pull request Sep 29, 2022
* Tdl 19964 add missing tap tester tests (#65)

* TDL-19964 Added missing tap-tester commits

* updated tap-tester tests

* updated bookmarks tap-tester test

* Updated and removed extra comments

* updated start date as per format

* updated pagination test case

* resolved review comments

* added new stream in tap-tester

* fixed the cci issues

* added missing assertion for all fields

* removed get_logger()

* added logger instead of print

* TDL-19957 Update dict based to class based implementation (#64)

* TDL-19957 update dict based to class based

* updated while condition

* updated while condition

* removed incremental_range from REQUIRED_CONFIG_KEYS

* updated discover and schema file

* updated to replication_key instead of keys

* updated schemas and added comments

* added unittests for code coverage

* added unittests for sync.py

* added parameterized in setup.py

* added parameterized in config.yml and updated unittests

* updated the replication key to list instead of a single key

* fixed the issue for Keyerror of form_id in answers stream

* updated unittests

* added configurable page size param

* handled page_size for 0 and updated unittests

* resolved bug fixes for pagination and removed incompleted_forms_only param

* added new stream unsubmitted landings

* resolved PR review comments

* added page_size in example config

* added back incremental_range in the tap-tester

* raised exc instead of fatal error message and updated unittests

* resolved PR comments

* fixed the issue when page_size not passed in config

* TDL-19801: Tap fetch data for sub-questions (#62)

* TDL-19801: Tap does not support fetching data for the questions nested within a Question Group.

* addressed the comments

* add unittest

* modify funciton comment

* formated test_case value in unittest

* formated expected_case

* Updated schema in questions.json

* resolved build fail error

* resolve build fail error

* updated unittest

* updated setup.py

* add parameterized

* change start date in start_date_test

* make change in bookmark test

* Updated unit test case.

* Updated schemas and keyerror.

Co-authored-by: Jay Tilala <jay.tilala@CDSYS.LOCAL>
Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com>

* TDL-19959 added missing fields (#63)

* added missing fields

* TDL-19957 update dict based to class based

* updated while condition

* updated while condition

* removed incremental_range from REQUIRED_CONFIG_KEYS

* TDL-19964 Added missing tap-tester commits

* updated discover and schema file

* updated tap-tester tests

* updated to replication_key instead of keys

* updated schemas and added comments

* added unittests for code coverage

* updated bookmarks tap-tester test

* added unittests for sync.py

* added parameterized in setup.py

* added parameterized in config.yml and updated unittests

* updated the replication key to list instead of a single key

* Updated and removed extra comments

* fixed the issue for Keyerror of form_id in answers stream

* updated unittests

* updated start date as per format

* added configurable page size param

* updated pagination test case

* handled page_size for 0 and updated unittests

* resolved review comments

* resolved bug fixes for pagination and removed incompleted_forms_only param

* updated array type schema

* added new stream unsubmitted landings

* added new stream in tap-tester

* updated indentation

* resolved PR review comments

* updated indentation to use 2 spaces

* added page_size in example config

* added back incremental_range in the tap-tester

* fixed the cci issues

* added missing fields to a dict

* added missing assertion for all fields

* updated comment

* raised exc instead of fatal error message and updated unittests

* removed get_logger()

* added logger instead of print

* Updated schema for questions.

* Removed duplicate assertion in all_fields test.

Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com>

* Updated schema for answers stream.

* update version and changelog

* changelog update for v2.0.0

* changelog update for v2.0.0

* minor changelog update

Co-authored-by: namrata270998 <75604662+namrata270998@users.noreply.github.com>
Co-authored-by: jtilala <104966482+jtilala@users.noreply.github.com>
Co-authored-by: Jay Tilala <jay.tilala@CDSYS.LOCAL>
Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com>
Co-authored-by: kethan1122 <kcherukuri@talend.com>
Co-authored-by: rdeshmukh15 <107538720+rdeshmukh15@users.noreply.github.com>
@kethan1122 kethan1122 deleted the TDL-19957-update-dict-based-to-class-based branch April 28, 2023 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants