🚨 Source Appsflyer: Migrate to V2 Appsflyer API
What
Migrate from obsolete V1 API to V2. Fixes #37403, #35667, #32446.
How
Appsflyer has deprecated & removed the V1 APIs. This PR migrates the connector to the API V2.
Review guide
User Impact
Users will have to replace the API V1 tokens with API V2 tokens as described here.
Can this PR be safely reverted and rolled back?
- [ ] YES 💚
- [ ] NO ❌
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| airbyte-docs | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Jun 7, 2024 3:20pm |
@AnjayGoel thank you for contributing! Running CI on this now.
I'd like to remove the dockerfile and setup.py while we're here — would you mind rebasing if/when I ping you?
@AnjayGoel thank you for contributing! Running CI on this now.
I'd like to remove the dockerfile and setup.py while we're here — would you mind rebasing if/when I ping you?
Sure
Time to rebase! #38363 is in.
Time to rebase! #38363 is in.
Done
Is this connector running for all the streams? I think there are some schema/columns changes on the definition of the output. So maybe it would be nice to check and apply those changes. Here you have a reference about what I am talking about https://github.com/aoelvp94/airbyte/commit/11d984ad817401f3581e0172b86177e4db1c7a06
Is this connector running for all the streams? I think there are some schema/columns changes on the definition of the output. So maybe it would be nice to check and apply those changes. Here you have a reference about what I am talking about aoelvp94@11d984a
Hi @aoelvp94, thanks for pointing this out. It looks like some streams (geo_report & partners_report) have additional properties, based on the custom events the user sends to AppsFlyer (counter/unique users fields). So, we have to move to a dynamic schema for these streams. I will try to make those changes and update the PR soon
Hi @aoelvp94 I've made the necessary changes to move to a dynamic schema. I've also tested all the streams, and everything seems to work fine.
Hi @aoelvp94 I've made the necessary changes to move to a dynamic schema. I've also tested all the streams, and everything seems to work fine.
Going to check from my side. I will let you know
Looks good so far — I'll wait for other feedback, then we can apply formatters and see if we're ready to merge.
One wild question: is AppsFlyer just a REST api under the hood? Should we yolo rewrite this to a low-code connector instead?
Yes, AppsFlyer is a REST API. If we can handle rate limiting & parse CSV responses then I think we should be able to rewrite this with the low-code connector instead.
Hi. Which streams are going to be supported by this patch? Because I guess we are just fixing existing streams, not adding new ones by the updated API?
Correct me if I am wrong
Then I have another question. Can the dynamic schema infer data types from csv? Or it just finds the values. I don't understand why if that is the case why the original code have a static schema in the first place
Hi @arielsnowik, The PR fixes the existing streams. I am not entirely sure if some new endpoints were introduced in v2. However, I have noticed that some endpoints, such as reinstalls, the pull API for organic source, and the ad-revenue data API, are missing in the current connector.
Also, The dynamic schema is only needed for the aggregate data APIs due to user-defined events present in the API response. It's not inferring the data types; it's just finding the additional fields and adding them to the schema as numerical type since the aggregate report only contains numbers.
Thank you for the response @AnjayGoel
We are analyzing using this patch (or to make our one) for a business project. Might be useful to understand which endpoints will and which ones will not be supported (so we know what to develop if we need them), may we help with that?
About the dynamic schema, so I understand we have a mix between static schema and dynamic schema for different fields, In our version of the patch we were trying to just make all static for simplicity of the solution, I don't know which would be better for long term support, because CSV does not support in a native way dynamic schema I believe (so making it dynamic sounds to me a little over complicated I believe?) correct me if I am wrong in my reasoning.
Let me know what we can do to help you to make this patch complete/ well documented, as I said we are evaluating using/contributing to this patch or just making our own.
Hi @arielsnowik, Following APIs are supported as of now:
- Aggregated Data API (Except Reinstalls)
- Raw Data API (Non Organic)
- Raw Data(Retargeting)
Remaining ones are:
Adding support for them should be a simple task.
Also the issue with dynamic schema is because AppsFlyer is aggregating the In-App Events Statistics in the same report. The format of the aggregated report is like this:
Date, Country,Agency/PMD (af_prt),Media Source (pid),Campaign (c),Impressions,Clicks,CTR,Installs,Conversion Rate,Sessions,Loyal Users,Loyal Users/Installs,Total Revenue,Total Cost,ROI,ARPU,Average eCPI,in_app_event_1 (Unique users),in_app_event_1 (Event counter),in_app_event_1 (Sales in USD),in_app_event_2 (Unique users),in_app_event_2 (Event counter),in_app_event_2 (Sales in USD)...
Since everyone will have different In-App Events, its not possible to have a static schema definition. Also the events may be added/removed, changing the report. If we want to keep the schemas static, we can create a different stream for events statistics like this:
Date, Country,Agency/PMD (af_prt),Media Source (pid),Campaign (c), Event Name, Event counter,Sales in USD,Unique users
Excellent @AnjayGoel. Thank you for the information about available methods. Would be nice to have that on readme if it is not already
I get it. In app events does not depend on the input, just the user, thus there the need dynamic to make thing generic. (Would have been nice to have read that in appsflyer docs...)
However I believe also your idea for a static schema using additional stream is nice and intuitive, +1. Let me know what we can do to help.
Kind Regards
Hi @arielsnowik, Thanks for the feedback. I will go ahead and add the additional stream, as discussed, soon. I will also document the currently supported endpoints.
Hi. @AnjayGoel I have another question. When you refer to aggregated data api Reinstalls (not implemented), I am not finding it on appsflyer docs
https://dev.appsflyer.com/hc/reference/overview-11
May I be looking wrong at docs? Or maybe it does not exist anymore?
Hi. @AnjayGoel I have another question. When you refer to aggregated data api Reinstalls (not implemented), I am not finding it on appsflyer docs https://dev.appsflyer.com/hc/reference/overview-11
May I be looking wrong at docs? Or maybe it does not exist anymore?
My bad, Its in Raw Data API
Hi @arielsnowik, I have added the streams and updated the readme.
Hi @natikgadzhi , @aoelvp94, Please let me know if anything else needs to be done before merging this PR.
Hi @arielsnowik, I have added the streams and updated the readme.
Nice, thank you!!
As long as the connector is properly tested and there is a clear way (by just reading readme...) for anyone to implement missing endpoints I think it would be ok to merge the PR. Let me know what do you (and the others) think.
Kind Regards
lgtm, @natikgadzhi and @marcosmarxm how is it possible to continue with this development?
/format-fix
Format-fix job started... Check job output.
✅ Changes applied successfully. (dda0fdee41ff8d4d2abbc2d27b72576d4e6a794f)
Kicking off CI on this — I am not certain if we have test creds, but if the idea is that the current version is broken, than I just need formatters and static checks to pass.
@marcosmarxm, this one needs a breaking change migration guide:
source-appsflyer - ❌ Failed - Breaking changes must be accompanied by a migration guide: Migration guide file is missing for source-appsflyer. Please create a migration guide in ./docs/integrations/<connector-type>s/<connector-name>-migrations.md`.
source-appsflyer - ❌ Failed - Breaking change deadline should be a week in the future: The upgrade deadline for the breaking changes in 0.2.0 is less than 7 days from today. Please extend the deadline.
source-appsflyer - ❌ Failed - Connector version in metadata.yaml and pyproject.toml file must match: Version is 0.2.0 in metadata.yaml, but version is 0.1.1 in pyproject.toml. These two files have to be consistent.
Error: 3 checks failed
Can you link breaking change migration guide docs so @AnjayGoel can write one?
@AnjayGoel, this also needs version to be set in both pyproject.toml and metadata.
@natikgadzhi I have made the changes
Triggered CI Tests.
Hello your feedback matters a lot to us. Can you spare just 3 minutes to complete a survey? We're dedicated to making the contributor experience even better, and your input is key to achieving excellence. Thank you for helping us improve!
