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

Fix negative timestamp #24

Merged
merged 5 commits into from
Feb 19, 2021
Merged

Fix negative timestamp #24

merged 5 commits into from
Feb 19, 2021

Conversation

cosimon
Copy link
Contributor

@cosimon cosimon commented Feb 19, 2021

Copied from #22. Moved into branch on this repo so circleci would run.

Description of change

API returns negative timestamps on occasion. Technically, a negative timestamp value is not invalid as it is simply a date prior to 01-01-1970 but it is likely that this is just bad data being sent by the Intercom API as one of the examples was sending back a negative timestamp for the signed_up_at field. Furthermore, the API is also inconsistent about sending timestamps in unix seconds or unix milliseconds so the get_integer_places function was designed to try and determine the units of the timestamp. This is where passing in a negative timestamp would cause the program to fail because the function tries to take the log of a value but you cannot take the log of a negative number. This fix only prevents the program from failing when given a negative timestamp but does not try to determine if the timestamp is in seconds or milliseconds as 1) it would not be guaranteed anyway and 2) it is likely bad data anyway. Furthermore, unit tests are included to ensure that the get_integer_places function will properly handle negative timestamps and that these negative timestamps will be properly converted to string dates downstream.

Manual QA steps

  • Run python -m nose to run unit tests.

Risks

  • Negative timestamp could affect a different branch of logic.

Rollback steps

  • revert this branch

@cosimon cosimon merged commit f7b142a into master Feb 19, 2021
@cosimon cosimon deleted the fix-negative-timestamp branch February 19, 2021 19:13
@cosimon cosimon mentioned this pull request Feb 19, 2021
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.

2 participants