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

Add Singer Facebook Source #808

Merged
merged 22 commits into from Nov 5, 2020
Merged

Add Singer Facebook Source #808

merged 22 commits into from Nov 5, 2020

Conversation

sherifnada
Copy link
Contributor

@sherifnada sherifnada commented Nov 4, 2020

What

Adds the FB singer-based source.

Accompanying docs are in #809

I'm not adding this to PR tests because FB heavily throttles the default token type. We have to apply for access to get the least-throttled token type, which is currently in progress.

checklist

  • publish integration docker image

@sherifnada sherifnada changed the title Sherif/fb ads source Add Singer Facebook Source Nov 4, 2020
Copy link
Contributor

@cgardens cgardens left a comment

Choose a reason for hiding this comment

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

how did you test this if you don't have the api key yet?

command = "install .[tests]"
}

task unitTest(type: PythonTask){
Copy link
Contributor

Choose a reason for hiding this comment

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

when do we use this versus command = "setup.py test"? it seems like we do both things.

Copy link
Contributor

Choose a reason for hiding this comment

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

also curious about this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we only give guidance to use pytest (or python -m pytest, they're 99% equivalent) now because setup.py test is deprecated.

…singer/setup.py

Co-authored-by: Charles <giardina.charles@gmail.com>
@sherifnada
Copy link
Contributor Author

sherifnada commented Nov 4, 2020

@cgardens I used the dev-tier access token to run tests. But it is very throttled. The cap varies based on which resource we're hitting but we're basically going to hit after a couple of PRs per hour. I'm applying to the better access tier today.

@sherifnada sherifnada merged commit f7b9bda into master Nov 5, 2020
@sherifnada sherifnada deleted the sherif/fb-ads-source branch November 5, 2020 00:39
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.

None yet

4 participants