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.

Sign up for GitHub

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

Jump to bottom

TDL-14798: Add a proper error message for timezone issue #35

Closed
wants to merge 22 commits into from
Closed

TDL-14798: Add a proper error message for timezone issue #35

wants to merge 22 commits into from

Conversation

savan-chovatiya
Copy link
Contributor

@savan-chovatiya savan-chovatiya commented Sep 3, 2021 •
edited
Loading

Description of change

TDL-14798: TimeZones appear to break customer syncs

  • As per finalized approach, added error handling as part of this ticket, as we can not fetch timezone via API.

Manual QA steps

  • Verified that proper error message is raised in case of timezone-related API failure.

Risks

Rollback steps

  • revert this branch

dbshah1212 and others added 5 commits September 2, 2021 11:22
@dbshah1212
Tdl 14794 implement tap tester tests (#30)
c95c7c0 
* TDL-14794: Added new tep-tester test cases

* TDL-14794: Updated CCI

* TDL-14794: Fixed pagination bug

* TDL-14794: Updated test path

* TDL-14794: Removed unused imports

* TDL-14794: Removed unused import

* TDL-14794: Formated the code

* TDL-14794: Removed page size

* TDL-14794: Added backoff for 429

* Test: Put loggers

* TDL-14794: Updated the error handling of tap

* TDL-14794: Put latest start date

* TDL-14794: Updated test cases with end_date

* TDL-14794: Made replication_keys automatic

* TDL-14794: Resolved review comments

* TDL-14794: Updated start date

* TDL-14794: Updated test cases to test more scenarios

* TDL-14794: Added Unit tests and all_filed tests cases in pagination test

* TDL-14794: Updated pytest to nose

* TDL-14794: Updated parent class of 500 error

Co-authored-by: dbshah1212 <root@ubuntu16.cdsys.local>
@dbshah1212
Tdl 14795 streams and sync should not be dict based (#32)
01a3f5b 
* TDL-14795: Changed stream and sync file structure from dict base to class base

* TDL-14795: Removed unused imports

* TDL-14795: Updated replication key logic

* TDL-14795: Converted code from CLRF to LF

* TDL-14795: Optimized the code

Co-authored-by: dbshah1212 <root@ubuntu16.cdsys.local>
@dbshah1212
Tdl 14795 streams and sync should not be dict based (#33)
8b95ad9 
* TDL-14795: Changed stream and sync file structure from dict base to class base

* TDL-14795: Removed unused imports

* TDL-14795: Updated replication key logic

* TDL-14795: Converted code from CLRF to LF

* TDL-14795: Optimized the code

* TDL-14795: Updated code to make compaitable with new tap code structure

* TDL-14795: Removed res files

Co-authored-by: dbshah1212 <root@ubuntu16.cdsys.local>
@dbshah1212
Tdl 14794 implement tap tester tests (#34)
2824ac1 
* TDL-14794: Added new tep-tester test cases

* TDL-14794: Updated CCI

* TDL-14794: Fixed pagination bug

* TDL-14794: Updated test path

* TDL-14794: Removed unused imports

* TDL-14794: Removed unused import

* TDL-14794: Formated the code

* TDL-14794: Removed page size

* TDL-14794: Added backoff for 429

* Test: Put loggers

* TDL-14794: Updated the error handling of tap

* TDL-14794: Put latest start date

* TDL-14794: Updated test cases with end_date

* TDL-14794: Made replication_keys automatic

* TDL-14794: Resolved review comments

* TDL-14794: Updated start date

* TDL-14794: Updated test cases to test more scenarios

* TDL-14794: Added Unit tests and all_filed tests cases in pagination test

* TDL-14794: Updated pytest to nose

* TDL-14794: Updated parent class of 500 error

* TDL-14794: Removed content length as Andy suggested in github

Co-authored-by: dbshah1212 <root@ubuntu16.cdsys.local>
@savan-chovatiya
TSL-14798: Add a proper error message for timezone issue
7d95dc4
@karanpanchal-crest karanpanchal-crest changed the base branch from crest-develop to master September 24, 2021 10:04
@karanpanchal-crest karanpanchal-crest changed the base branch from master to crest-develop September 24, 2021 10:04
karanpanchal-crest
karanpanchal-crest reviewed Sep 24, 2021
View reviewed changes
tap_mixpanel/client.py Outdated
error_code, {})).get(
"message", "Unknown Error"))

# if response text contains something unusual errpr of to_date then provide helper message of timezone mismatch
Copy link

Choose a reason for hiding this comment

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

@savan-chovatiya Resolve the spelling mistake in the comment for word 'error'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved spell mistake.

karanpanchal-crest
karanpanchal-crest reviewed Sep 24, 2021
View reviewed changes
tap_mixpanel/client.py Outdated
if error_code == 400 and "to_date" in response.text:
error_message += " Please validate the timezone with the MixPanel UI under project settings."

message = "HTTP-error-code: {}, Error: {}".format(
Copy link

Choose a reason for hiding this comment

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

@savan-chovatiya As per the latest best practices, please use f-strings instead of format

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 formatted message with f-strings.

karanpanchal-crest
karanpanchal-crest reviewed Sep 24, 2021
View reviewed changes
tests/unittests/test_error_handling.py Outdated

# Mock response
def get_mock_http_response(content, status_code):
contents = content
Copy link

Choose a reason for hiding this comment

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

@savan-chovatiya No need of this variable. You can directly use 'content' variable at line 13

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 contents variable and directly used content.

@savan-chovatiya savan-chovatiya changed the base branch from crest-develop to master September 27, 2021 07:28
@savan-chovatiya savan-chovatiya changed the base branch from master to crest-develop September 27, 2021 07:30
@dbshah1212 dbshah1212 changed the base branch from crest-develop to master September 27, 2021 08:45
dbshah1212 and others added 3 commits October 27, 2021 11:50
@dbshah1212 @savan-chovatiya @prijendev
Tdl 14840 fix date time conversions (#45)
ab32ecd 
* TDL-14840: Updated transform to fix conversion

* Tdl 14794 implement tap tester tests (#30)

* TDL-14794: Added new tep-tester test cases

* TDL-14794: Updated CCI

* TDL-14794: Fixed pagination bug

* TDL-14794: Updated test path

* TDL-14794: Removed unused imports

* TDL-14794: Removed unused import

* TDL-14794: Formated the code

* TDL-14794: Removed page size

* TDL-14794: Added backoff for 429

* Test: Put loggers

* TDL-14794: Updated the error handling of tap

* TDL-14794: Put latest start date

* TDL-14794: Updated test cases with end_date

* TDL-14794: Made replication_keys automatic

* TDL-14794: Resolved review comments

* TDL-14794: Updated start date

* TDL-14794: Updated test cases to test more scenarios

* TDL-14794: Added Unit tests and all_filed tests cases in pagination test

* TDL-14794: Updated pytest to nose

* TDL-14794: Updated parent class of 500 error

Co-authored-by: dbshah1212 <root@ubuntu16.cdsys.local>

* Tdl 14795 streams and sync should not be dict based (#32)

* TDL-14795: Changed stream and sync file structure from dict base to class base

* TDL-14795: Removed unused imports

* TDL-14795: Updated replication key logic

* TDL-14795: Converted code from CLRF to LF

* TDL-14795: Optimized the code

Co-authored-by: dbshah1212 <root@ubuntu16.cdsys.local>

* Tdl 14795 streams and sync should not be dict based (#33)

* TDL-14795: Changed stream and sync file structure from dict base to class base

* TDL-14795: Removed unused imports

* TDL-14795: Updated replication key logic

* TDL-14795: Converted code from CLRF to LF

* TDL-14795: Optimized the code

* TDL-14795: Updated code to make compaitable with new tap code structure

* TDL-14795: Removed res files

Co-authored-by: dbshah1212 <root@ubuntu16.cdsys.local>

* Tdl 14794 implement tap tester tests (#34)

* TDL-14794: Added new tep-tester test cases

* TDL-14794: Updated CCI

* TDL-14794: Fixed pagination bug

* TDL-14794: Updated test path

* TDL-14794: Removed unused imports

* TDL-14794: Removed unused import

* TDL-14794: Formated the code

* TDL-14794: Removed page size

* TDL-14794: Added backoff for 429

* Test: Put loggers

* TDL-14794: Updated the error handling of tap

* TDL-14794: Put latest start date

* TDL-14794: Updated test cases with end_date

* TDL-14794: Made replication_keys automatic

* TDL-14794: Resolved review comments

* TDL-14794: Updated start date

* TDL-14794: Updated test cases to test more scenarios

* TDL-14794: Added Unit tests and all_filed tests cases in pagination test

* TDL-14794: Updated pytest to nose

* TDL-14794: Updated parent class of 500 error

* TDL-14794: Removed content length as Andy suggested in github

Co-authored-by: dbshah1212 <root@ubuntu16.cdsys.local>

* TDL-14840: Resolved merge conflicts

* TDL-14840: Removed extra space

* TDL-14840: Removed extra line

* Added coverage report to artifact0

* Updated artifact sequence

* Added code comment

Co-authored-by: savan-chovatiya <savan.chovatiya@crestdatasys.com>
Co-authored-by: dbshah1212 <root@ubuntu16.cdsys.local>
Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com>
Co-authored-by: prijendev <88327452+prijendev@users.noreply.github.com>
@dbshah1212 @savan-chovatiya
@namrata270998 @prijendev
Tdl 6141 switch multiple of to singer decimal (#46)
242d6c0 
* Tdl 14794 implement tap tester tests (#30)

* TDL-14794: Added new tep-tester test cases

* TDL-14794: Updated CCI

* TDL-14794: Fixed pagination bug

* TDL-14794: Updated test path

* TDL-14794: Removed unused imports

* TDL-14794: Removed unused import

* TDL-14794: Formated the code

* TDL-14794: Removed page size

* TDL-14794: Added backoff for 429

* Test: Put loggers

* TDL-14794: Updated the error handling of tap

* TDL-14794: Put latest start date

* TDL-14794: Updated test cases with end_date

* TDL-14794: Made replication_keys automatic

* TDL-14794: Resolved review comments

* TDL-14794: Updated start date

* TDL-14794: Updated test cases to test more scenarios

* TDL-14794: Added Unit tests and all_filed tests cases in pagination test

* TDL-14794: Updated pytest to nose

* TDL-14794: Updated parent class of 500 error

Co-authored-by: dbshah1212 <root@ubuntu16.cdsys.local>

* Tdl 14795 streams and sync should not be dict based (#32)

* TDL-14795: Changed stream and sync file structure from dict base to class base

* TDL-14795: Removed unused imports

* TDL-14795: Updated replication key logic

* TDL-14795: Converted code from CLRF to LF

* TDL-14795: Optimized the code

Co-authored-by: dbshah1212 <root@ubuntu16.cdsys.local>

* Tdl 14795 streams and sync should not be dict based (#33)

* TDL-14795: Changed stream and sync file structure from dict base to class base

* TDL-14795: Removed unused imports

* TDL-14795: Updated replication key logic

* TDL-14795: Converted code from CLRF to LF

* TDL-14795: Optimized the code

* TDL-14795: Updated code to make compaitable with new tap code structure

* TDL-14795: Removed res files

Co-authored-by: dbshah1212 <root@ubuntu16.cdsys.local>

* Tdl 14794 implement tap tester tests (#34)

* TDL-14794: Added new tep-tester test cases

* TDL-14794: Updated CCI

* TDL-14794: Fixed pagination bug

* TDL-14794: Updated test path

* TDL-14794: Removed unused imports

* TDL-14794: Removed unused import

* TDL-14794: Formated the code

* TDL-14794: Removed page size

* TDL-14794: Added backoff for 429

* Test: Put loggers

* TDL-14794: Updated the error handling of tap

* TDL-14794: Put latest start date

* TDL-14794: Updated test cases with end_date

* TDL-14794: Made replication_keys automatic

* TDL-14794: Resolved review comments

* TDL-14794: Updated start date

* TDL-14794: Updated test cases to test more scenarios

* TDL-14794: Added Unit tests and all_filed tests cases in pagination test

* TDL-14794: Updated pytest to nose

* TDL-14794: Updated parent class of 500 error

* TDL-14794: Removed content length as Andy suggested in github

Co-authored-by: dbshah1212 <root@ubuntu16.cdsys.local>

* TDL-6141: Switched multipleOf with singer.decimal

* TDL-6141 removed the whitespaces

* TDL-6141 removed the multipleOf from schema.py

* TDL-6141 removed a whitespace

* TDL-6141: Resolved merge conflicts

* Added coverage report to artifact

* Updated circleci config.yml

Co-authored-by: dbshah1212 <root@ubuntu16.cdsys.local>
Co-authored-by: savan-chovatiya <savan.chovatiya@crestdatasys.com>
Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>
Co-authored-by: namrata270998 <75604662+namrata270998@users.noreply.github.com>
Co-authored-by: prijendev <88327452+prijendev@users.noreply.github.com>
Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com>
@dbshah1212
Merge branch 'TDL-14784-mixpanel-all-code' into TDL-14798-fix-timezon…
be46d80 
…e-issue-in-api
@dbshah1212 dbshah1212 mentioned this pull request Oct 27, 2021
Merged
@prijendev
Copy link
Contributor

The code change of this PR is already merged into the main branch. Hence, closing this PR.

@prijendev prijendev closed this Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karanpanchal-crest karanpanchal-crest karanpanchal-crest approved these changes

@KrisPersonal KrisPersonal KrisPersonal approved these changes

@dbshah1212 dbshah1212 Awaiting requested review from dbshah1212

@luandy64 luandy64 Awaiting requested review from luandy64

@zachharris1 zachharris1 Awaiting requested review from zachharris1

@cosimon cosimon Awaiting requested review from cosimon

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@savan-chovatiya @prijendev @karanpanchal-crest @KrisPersonal @dbshah1212 @namrata270998