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-17002: Make the logger messages more descriptive #46

Merged
merged 5 commits into from
Jan 5, 2022

Conversation

hpatel41
Copy link
Contributor

Description of change

TDL-17002: Make the logger messages more descriptive

  • Added logger messages

Manual QA steps

Risks

Rollback steps

  • revert this branch

@hpatel41 hpatel41 marked this pull request as ready for review December 30, 2021 13:00
else:
scrolling = False

records = transform_json(response, self.tap_stream_id, self.data_key)

Choose a reason for hiding this comment

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

Why do we remove this?

Copy link
Contributor Author

@hpatel41 hpatel41 Dec 30, 2021

Choose a reason for hiding this comment

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

We did not remove it, just shifted above the condition as it contains the records, we can print the logger containing the number of records and then the logger of "Syncing next page" if the response contains the next page value.

@hpatel41 hpatel41 changed the base branch from master to crest-master December 31, 2021 05:55
@dbshah1212 dbshah1212 self-requested a review December 31, 2021 10:30
tap_intercom/streams.py Outdated Show resolved Hide resolved
tap_intercom/streams.py Outdated Show resolved Hide resolved
@hpatel41 hpatel41 requested a review from dmosorast December 31, 2021 15:02
tap_intercom/streams.py Outdated Show resolved Hide resolved
tap_intercom/streams.py Outdated Show resolved Hide resolved
@hpatel41 hpatel41 requested a review from dmosorast January 4, 2022 04:42
@@ -223,10 +222,10 @@ class Admins(FullTableStream):
parent = AdminList

def get_records(self, bookmark_datetime=None, is_parent=False) -> Iterator[list]:
LOGGER.info("Syncing: {}".format(self.parent.tap_stream_id))
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be self.tap_stream_id instead of self.parent.tap_stream_id. Apologies for being so particular, but these kinds of log lines can cause a large amount of confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to self.tap_stream_id

@hpatel41 hpatel41 requested a review from dmosorast January 4, 2022 14:31
@hpatel41 hpatel41 merged commit 602f913 into crest-master Jan 5, 2022
@hpatel41 hpatel41 mentioned this pull request Jan 5, 2022
KrisPersonal pushed a commit that referenced this pull request Jan 5, 2022
* TDL-17000: fix breaking changes and TDL-17017: Fix bookmark for conversation_parts stream. (#45)

* Reverted conversation_parts to full_table for parent as V1.1.3

* Updated bookmark test to handle conversation_parts

* Updated comments

* Added code to select parent_stream if only child stream is selected same as v1.1.3

* Added code comments

* Resolved review comment

* Resolved review comment

* Added code comments

* Added bookmark mechanism for conversation_parts same as before but independant of conversations

* Added code comments

* Added query paramater to get conversations greater and equal to bookmark

* Adding changes of PR42 for better unit tests

* Added logger messages

* Added unit test

* Updated unit test

* Updated unit test

* Updated variables in unit test

* Added time_extracted for conversation_parts

* Resolved review comments

* Added companies to untestable for bookmark test

* Reverted changes of last commit

* Tdl 17003 Added back missing field for contact streams (#43)

* Reverted the logic as per old version

* Removed leads as well as it is not supported by tap

* Tdl 17006 revert back companies to incremental (#44)

* Reverted companies to incremental stream

* Debuging integration test

* Removed print statement

* Added companies to untestable for bookmark test

* TDL-17002: Make the logger messages more descriptive (#46)

* added logger messages

* removed unnecessary loggers

* removed unnecessary loggers

* updated the code to use stream name instead of parent stream name

Co-authored-by: savan-chovatiya <80703490+savan-chovatiya@users.noreply.github.com>
Co-authored-by: Umang Agrawal <80704207+umangagrawal-crest@users.noreply.github.com>
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.

4 participants