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

Facebook ads datetime object conversion fix. #244

Merged
merged 4 commits into from
Aug 18, 2023
Merged

Facebook ads datetime object conversion fix. #244

merged 4 commits into from
Aug 18, 2023

Conversation

zem360
Copy link
Contributor

@zem360 zem360 commented Aug 14, 2023

Tell us what you do here

  • implementing verified source (please link a relevant issue labelled as verified source)
  • fixing a bug (please link a relevant bug report)
  • improving, documenting, customizing existing source (please link an issue or describe below)
  • anything else (please link an issue or describe below)

More PR info

The helper function get_start_date() is using the parse_iso_like_datetime() function to convert the ISO datetime string into pendulum.datetime object. The parse_iso_like_datetime() function can not handle ISO date string and returns an datetime.date object instead of a pendulum.datetime object.

I replace the parse_iso_like_datetime() with ensure_pendulum_datetime function inside the get_start_date(). I also wrote some test to ensure that pendulum.datetime is returned.

@zem360 zem360 requested a review from rudolfix August 14, 2023 15:24
Copy link
Contributor

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

it looks good, but your recent fix with breakdowns is not here. please rebase on current master branch and push again

@zem360
Copy link
Contributor Author

zem360 commented Aug 17, 2023

I rebased from master and have removed the breakdown parameter with new commit.

@zem360 zem360 requested a review from rudolfix August 17, 2023 19:40
@rudolfix rudolfix merged commit 5ab2f0f into master Aug 18, 2023
9 checks passed
@rudolfix rudolfix deleted the bug_fb_ads branch August 18, 2023 14:41
@rudolfix rudolfix restored the bug_fb_ads branch August 18, 2023 14:43
@zem360 zem360 deleted the bug_fb_ads branch September 20, 2023 13:01
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