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

[Singer][FacebookAds] Pipelines with the field include deleted set to true or false failing #49

Closed
2 of 8 tasks
rafaelsantanaep opened this issue Dec 28, 2022 · 7 comments
Labels
bug: moderate Results unexpecte behavior, not enough to disrupt system function but block the customer usability Sprint 1 (02/01/23-13/01/23) Sprint 1 (02/01/23-13/01/23) Sprint 26 unplanned added in the middle of the sprint

Comments

@rafaelsantanaep
Copy link
Collaborator

rafaelsantanaep commented Dec 28, 2022

Mandatory information:

There are customers directly impacted by this bug. Which?

None, but it may impact duxnutritioncom depending on how Vinicius creates the Facebook Ads pipeline.

Bug Category

  • Connection Manager
  • Connection Test
  • Creating Pipeline
  • Executing Pipeline
  • Cataloging Data Asset
  • Other
Describe the bug

By default, this field is set to None. This field is mapped as a boolean in brain, however, the tap is waiting for a string, as you can see in the code chunk below:

        if CONFIG.get('include_deleted', 'false').lower() == 'true':
            ads = do_request_multiple()
        else:
            ads = do_request()
        for message in self._iterate(ads, prepare_record):
            yield message

Logs

'bool' object has no attribute 'lower' Traceback (most recent call last): File "/app/.meltano/extractors/tap-facebook/venv/lib/python3.7/site-packages/tap_facebook/__init__.py", line 926, in main main_impl() File "/app/.meltano/extractors/tap-facebook/venv/lib/python3.7/site-packages/tap_facebook/__init__.py", line 917, in main_impl do_sync(account, catalog, args.state) File "/app/.meltano/extractors/tap-facebook/venv/lib/python3.7/site-packages/tap_facebook/__init__.py", line 806, in do_sync for message in stream: File "/app/.meltano/extractors/tap-facebook/venv/lib/python3.7/site-packages/tap_facebook/__init__.py", line 336, in __iter__ if CONFIG.get('include_deleted', 'false').lower() == 'true': AttributeError: 'bool' object has no attribute 'lower'

So, when this field is added to the pipeline, the jobs of the following entities will fail:

  • ads
  • adsets
  • campaigns

Two possible ways of fixing this issue:

  • Update the contract to receive a boolean. Requires modification of the payload sent by the front end and the required input in brain.
  • Update our fork to treat the field as booleans, not as strings. This requires diverging more from the canonical implementation of the tap-facebook.

Dadosfera Customer:


  • dadosfera

What environment of software are you using?

  • PRD
  • Other

When the bug happened::

  • 2022-12-28
@rafaelsantanaep
Copy link
Collaborator Author

rafaelsantanaep commented Dec 28, 2022

FYI: @beatrizaantunes @gabrielrra @samirleao @victorradael

@beatrizaantunes beatrizaantunes added Sprint 1 (02/01/23-13/01/23) Sprint 1 (02/01/23-13/01/23) unplanned added in the middle of the sprint bug: moderate Results unexpecte behavior, not enough to disrupt system function but block the customer usability labels Dec 28, 2022
@andersonmfjr
Copy link

andersonmfjr commented Jan 2, 2023

@rafaelsantanaep does the front end send the value as a string solve the problem? This is easy to do.

@andersonmfjr andersonmfjr self-assigned this Jan 2, 2023
@gabrielrra
Copy link
Contributor

Platform would still need to change the include_deleted type on their side, or else we will get a Bad Response when creating

@andersonmfjr
Copy link

andersonmfjr commented Jan 2, 2023

@andersonmfjr
Copy link

Update: the solution of converting the "include_deleted" data to string doesn't seem to have worked

@rafaelsantanaep
Copy link
Collaborator Author

rafaelsantanaep commented Jan 2, 2023

There was a bug in the meltano.yml and Meltano itself was forcing the string "true" to true and "false" to false. We had to override the settings from the tap in the following https://github.com/dadosfera/singer-connector/pull/260.

The pipeline that was created by @andersonmfjr was able to run successfully.

@beatrizaantunes
Copy link
Contributor

@samirleao tava vendo com o santana, essa parada foi resolvida já
valeu!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: moderate Results unexpecte behavior, not enough to disrupt system function but block the customer usability Sprint 1 (02/01/23-13/01/23) Sprint 1 (02/01/23-13/01/23) Sprint 26 unplanned added in the middle of the sprint
Projects
None yet
Development

No branches or pull requests

5 participants