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

🎉 Posthog: New Source #3768

Merged
merged 35 commits into from
Jun 8, 2021
Merged

🎉 Posthog: New Source #3768

merged 35 commits into from
Jun 8, 2021

Conversation

vovavovavovavova
Copy link
Contributor

@vovavovavovavova vovavovavovavova commented May 31, 2021 •
edited by keu
Loading

What

Closes #2010

How

Implemenation for this source. A lot of endpoints have no documented way to sort ASC, a lot of them have no documented filters by request arguments (like since, from_date etc), so most of endpoints supports only FullRefresh Mode.

Some of them, who are ordered Descendant (and paginated, like first page ids come from 500 to 400, second page from 400 to 300), and have no documented filter by date, is supported like "Reverse Incremental" - from the first element and until we reach the state value (save state only on reaching this value).

Pre-merge Checklist

  • Issue acceptance criteria met
  • PR name follows PR naming conventions
  • Secrets are annotated with airbyte_secret in output spec
  • Unit & integration tests added as appropriate (and are passing)
    • Community members: please provide proof of this succeeding locally e.g: screenshot or copy-paste acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • /test connector=connectors/<name> command as documented here is passing.
    • Community members can skip this, Airbyters will run this for you.
  • Code reviews completed
  • Credentials added to Github CI if needed and not already present. instructions for injecting secrets into CI.
  • Documentation updated
    • README
    • CHANGELOG.md
    • Reference docs in the docs/integrations/ directory.
  • Build is successful
  • Connector version bumped like described here
  • New Connector version released on Dockerhub by running the /publish command described here
  • No major blockers
  • PR merged into master branch
  • Follow up tickets have been created
  • Associated tickets have been closed & stakeholders notified

@auto-assign auto-assign bot requested review from cgardens and masonwheeler May 31, 2021 19:24
@vovavovavovavova
Copy link
Contributor Author

vovavovavovavova commented May 31, 2021 •
edited by github-actions bot
Loading

/test connector=source-posthog

🕑 source-posthog https://github.com/airbytehq/airbyte/actions/runs/893851936
❌ source-posthog https://github.com/airbytehq/airbyte/actions/runs/893851936

@vovavovavovavova vovavovavovavova changed the title Valdemar/#2010 new source posthog new source posthog May 31, 2021
@vovavovavovavova vovavovavovavova changed the title new source posthog Posthog: New Source May 31, 2021
yevhenii-ldv
yevhenii-ldv reviewed Jun 1, 2021
View reviewed changes
Copy link
Contributor

@yevhenii-ldv yevhenii-ldv left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@vovavovavovavova

  1. Please, use ./gradlew format for format your code, because now we have error with Format on Build step
  2. You should add env variable to the next files for run your CI Tests:
    • .github/workflows/test-command.yml
    • .github/workflows/publish-command.yml
    • tools/bin/ci_credentials.sh

Zirochkaa
Zirochkaa reviewed Jun 1, 2021
View reviewed changes
airbyte-integrations/connectors/source-posthog/source_posthog/spec.json Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-posthog/build.gradle Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-posthog/acceptance-test-docker.sh Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-posthog/CHANGELOG.md Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-posthog/source_posthog/source.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-posthog/source_posthog/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-posthog/source_posthog/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-posthog/source_posthog/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-posthog/source_posthog/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-posthog/source_posthog/streams.py Outdated Show resolved Hide resolved
@Zirochkaa
Copy link
Contributor

@vovavovavovavova

  1. Please, use ./gradlew format for format your code, because now we have error with Format on Build step

  2. You should add env variable to the next files for run your CI Tests:

    • .github/workflows/test-command.yml
    • .github/workflows/publish-command.yml
    • tools/bin/ci_credentials.sh

About first point - I checked format command on this branch locally and everything is ok. Build has a problem with python (see screenshot).
image

@vovavovavovavova
Copy link
Contributor Author

@vovavovavovavova

  1. Please, use ./gradlew format for format your code, because now we have error with Format on Build step

  2. You should add env variable to the next files for run your CI Tests:

    • .github/workflows/test-command.yml
    • .github/workflows/publish-command.yml
    • tools/bin/ci_credentials.sh

About first point - I checked format command on this branch locally and everything is ok. Build has a problem with python (see screenshot).
image

The gradle run is ok locally. Everything failes here due to (2) - absence of .github files. I will add them

@vovavovavovavova
Copy link
Contributor Author

vovavovavovavova commented Jun 1, 2021 •
edited by github-actions bot
Loading

/test connector=source-posthog

🕑 source-posthog https://github.com/airbytehq/airbyte/actions/runs/895838730
❌ source-posthog https://github.com/airbytehq/airbyte/actions/runs/895838730

@vovavovavovavova
Copy link
Contributor Author

vovavovavovavova commented Jun 1, 2021 •
edited by github-actions bot
Loading

/test connector=source-posthog

🕑 source-posthog https://github.com/airbytehq/airbyte/actions/runs/896314245
❌ source-posthog https://github.com/airbytehq/airbyte/actions/runs/896314245

@vovavovavovavova
Copy link
Contributor Author

vovavovavovavova commented Jun 1, 2021 •
edited by github-actions bot
Loading

/test connector=source-posthog

🕑 source-posthog https://github.com/airbytehq/airbyte/actions/runs/896469788
❌ source-posthog https://github.com/airbytehq/airbyte/actions/runs/896469788

@vovavovavovavova
Copy link
Contributor Author

vovavovavovavova commented Jun 1, 2021 •
edited by github-actions bot
Loading

/test connector=source-posthog

🕑 source-posthog https://github.com/airbytehq/airbyte/actions/runs/896964833
✅ source-posthog https://github.com/airbytehq/airbyte/actions/runs/896964833

@vovavovavovavova
Copy link
Contributor Author

vovavovavovavova commented Jun 8, 2021 •
edited by github-actions bot
Loading

/test connector=source-posthog

🕑 source-posthog https://github.com/airbytehq/airbyte/actions/runs/917391082
✅ source-posthog https://github.com/airbytehq/airbyte/actions/runs/917391082

@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Jun 8, 2021
keu
keu approved these changes Jun 8, 2021
View reviewed changes
Copy link
Contributor

@keu keu left a comment

Choose a reason for hiding this comment

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

agreed with @vovavovavovavova to do some refactoring manually

@keu keu requested review from sherifnada, davinchia and tuliren and removed request for htrueman, po3na4skld and vitaliizazmic June 8, 2021 22:16
@keu
Copy link
Contributor

keu commented Jun 8, 2021 •
edited by github-actions bot
Loading

/test connector=source-posthog

🕑 source-posthog https://github.com/airbytehq/airbyte/actions/runs/919835474
✅ source-posthog https://github.com/airbytehq/airbyte/actions/runs/919835474

sherifnada
sherifnada approved these changes Jun 8, 2021
View reviewed changes
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

LGTM with a request for one comment

airbyte-integrations/connectors/source-posthog/source_posthog/source.py
Annotations(authenticator=authenticator, start_date=config["start_date"]),
Cohorts(authenticator=authenticator),
Events(authenticator=authenticator, start_date=config["start_date"]),
# EventsSessions(authenticator=authenticator),
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a comment describing why this is commented out?

@keu keu changed the title Posthog: New Source 🎉 Posthog: New Source Jun 8, 2021
@keu
Copy link
Contributor

keu commented Jun 8, 2021 •
edited by github-actions bot
Loading

/publish connector=connectors/source-posthog

🕑 connectors/source-posthog https://github.com/airbytehq/airbyte/actions/runs/919997405
❌ connectors/source-posthog https://github.com/airbytehq/airbyte/actions/runs/919997405

@keu
Copy link
Contributor

keu commented Jun 8, 2021 •
edited by github-actions bot
Loading

/publish connector=connectors/source-posthog

🕑 connectors/source-posthog https://github.com/airbytehq/airbyte/actions/runs/920007716
✅ connectors/source-posthog https://github.com/airbytehq/airbyte/actions/runs/920007716

@sherifnada sherifnada merged commit 07b09a9 into master Jun 8, 2021
@sherifnada sherifnada deleted the valdemar/#2010_new_source_posthog branch June 8, 2021 23:55
@vovavovavovavova vovavovavovavova mentioned this pull request Aug 5, 2021
Merged
26 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yevhenii-ldv yevhenii-ldv yevhenii-ldv requested changes

@Zirochkaa Zirochkaa Zirochkaa left review comments

@sherifnada sherifnada sherifnada approved these changes

@keu keu keu approved these changes

@davinchia davinchia Awaiting requested review from davinchia

@tuliren tuliren Awaiting requested review from tuliren

Assignees
No one assigned
Labels
area/documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

New Source: Posthog
6 participants
@vovavovavovavova @Zirochkaa @keu @sherifnada @yevhenii-ldv @eugene-kulak