singer-io / tap-appsflyer

A Singer.io tap for extracting data from the AppsFlyer API
GNU Affero General Public License v3.0
11 stars 40 forks source link

Add organic intallations #20

Closed yonidavidson closed 3 years ago

yonidavidson commented 5 years ago

This will add organic installations if set in the config file. fixes #19

cmerrick commented 5 years ago

Hi @yonidavidson, thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

cmerrick commented 5 years ago

You did it @yonidavidson!

Thank you for signing the Singer Contribution License Agreement.

yonidavidson commented 5 years ago

Hi, anyone is looking at my PR? @cmerrick

yonidavidson commented 4 years ago

Do you have an example of how to fix it?

Yoni Davidson,

On Wed, Sep 2, 2020 at 10:08 PM ebeale-stitch notifications@github.com wrote:

@ebeale-stitch commented on this pull request.

In tap_appsflyer/init.py https://github.com/singer-io/tap-appsflyer/pull/20#discussion_r482319394 :

@@ -427,6 +554,9 @@ def sync_in_app_events(): def get_streams_to_sync(streams, state): target_stream = state.get("this_stream") result = streams

  • if "organic_installs" in CONFIG:
  • if CONFIG["organic_installs"]:
  • result.append(Stream("organic_installs", sync_organic_installs))

@yonidavidson https://github.com/yonidavidson I've been made aware that this change cannot be used with in Stitch because our product requires stream selection to happen in the catalog v. the config.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/singer-io/tap-appsflyer/pull/20#pullrequestreview-481198393, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJYECRNOLKWGLNZIVFAWUTSD2J4LANCNFSM4HVBN33A .

bogaert commented 3 years ago

Hi all! I wondered what is preventing this PR from being merged?

yonidavidson commented 3 years ago

Hi @bogaert , as you can see , a review is required

gaberogan commented 3 years ago

@yonidavidson This PR was already approved by @jornesc, could you or someone else re-approve? Or is this repository abandoned?

yonidavidson commented 3 years ago

@gaberogan I am waiting for review, I am actually use the already merged fork in my company.