🎉 New Source: Adjust #16051
Merged
🎉 New Source: Adjust #16051
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
sbjorn
changed the title
sbjorn
force-pushed
the
feature/source-adjust
branch
from
2d52f67 to
ecca4ef
Compare
github-actions
bot
added
the
area/documentation
|
assigned to @tuanchris |
tuanchris
suggested changes
Aug 31, 2022
There was a problem hiding this comment.
Hi @sbjorn, I'm participating in the Airbyte Maintainers Program and am assigned to review your PR. The PR looks good so far. I left a couple of comments below, can you have a look?
The Airbyte team is working on retrieving the credentials for this source.
Plus, the acceptance test TestBasicRead is passing, but there's actually an error. Can you check configured_catalog.json to resolve the error?
cc @sajarin
airbyte-integrations/connectors/source-adjust/integration_tests/sample_config.json
Outdated
Show resolved
Hide resolved
| for k, v in list(row.items()): | ||
| if k in model: | ||
| type_ = model[k].type_ | ||
| else: # Additional user-provided metrics are assumed to be decimal |
There was a problem hiding this comment.
Is there a case when user-provided metric is not float? Should we have a fall back to string type?
There was a problem hiding this comment.
I have not identified such a case, but that does not mean there isn't one. Could setup a try...except block to fallback together with some logging.
| description: Adjust API key, see https://help.adjust.com/en/article/report-service-api-authentication | ||
| title: API Token | ||
| type: string | ||
| dimensions: |
There was a problem hiding this comment.
I recommend adding examples to be more user-friendly. An URL to the possible values that users can select can also be helpful
There was a problem hiding this comment.
Possible values for dimensions are given, so I don't see how an example would make things clearer. I agree that we could add a link to the documentation of dimensions to make it easier for the user to find more details on the various dimensions.
| title: Dimensions | ||
| type: array | ||
| uniqueItems: true | ||
| ingest_start: |
There was a problem hiding this comment.
Can we implement pattern here to validate user's date input
There was a problem hiding this comment.
The format is set to date which should limit input to a ISO8601 style date (did not actually test if this is enforced by the UI). I think that should be enough if the spec is followed: https://json-schema.org/understanding-json-schema/reference/string.html#dates-and-times
| metrics = self.config["metrics"] + self.config["additional_metrics"] | ||
| date = stream_slice[self.cursor_field] | ||
| return { | ||
| "date_period": ":".join([date, date]), # inclusive |
There was a problem hiding this comment.
I'm wondering why don't we pass in ingest_start:today?
There was a problem hiding this comment.
👍 Did not test this. Could be an improvement.
There was a problem hiding this comment.
Considering this further however, it may be better leaving this as is. If the synch fails in the middle of a large ingest, we would have a checkpoint that could be restarted from.
There was a problem hiding this comment.
I noticed that the Adjust API supports pagination. We still are waiting on the credentials, which should be created soon. If this is the case, I would recommend using the ingest_start:today pattern. The Airbyte Python SDK already supports dealing with caching and pagination: https://docs.airbyte.com/connector-development/cdk-python/http-streams/.
Let me know what you think.
airbyte-integrations/connectors/source-adjust/source_adjust/source.py
Outdated
Show resolved
Hide resolved
I can not see what part of I am wondering this because the test that outputs the suspected error is called |
|
Thanks @sbjorn. Everything else looks good for now. I will wait for the credentials https://github.com/airbytehq/airbyte-internal-issues/issues/837, and finish the review. |
|
@sbjorn can you post a screenshot of passing acceptance tests from your local branch? That should be all we need at this point to close this out and merge. Let me know if you have any questions or concerns. |
|
@sajarin Here you go - full integration test output: |
sajarin
approved these changes
Sep 21, 2022
|
/publish connector=connectors/source-adjust run-tests=false
if you have connectors that successfully published but failed definition generation, follow step 4 here |
|
@sbjorn, I don't have access to your PR, so I can't push the related changes that would fix the definition generation. Add the following to the top of the This should be above Airtable since we're going alphabetically in order here. Afterwards, return to root and run We're almost there! Let me know if you run into any issues. |
|
@sajarin I have updated the Unfortunately, I can not grant push access to this fork because it is attached to an organization's repository. |
|
Merged. Thanks for the PR @sbjorn and thanks for the review @tuanchris! |
This was referenced Sep 27, 2022
robbinhan
pushed a commit
to robbinhan/airbyte
that referenced
this pull request
Sep 29, 2022
* Initial version of the adjust source connector * source-adjust: add bootstrap.md * adjust-source: add setup guide * adjust-source: update integration READMEs * source-adjust: better stream name * source-adjust: fix sample conf * source-adjust: add spec order metadata * source-adjust: improve spec dimension doc * source-adjust: warn on custom metric cast failure * source adjust: Update source_definitions.yaml * source adjust: Update documentation url
jhammarstedt
pushed a commit
to jhammarstedt/airbyte
that referenced
this pull request
Oct 31, 2022
* Initial version of the adjust source connector * source-adjust: add bootstrap.md * adjust-source: add setup guide * adjust-source: update integration READMEs * source-adjust: better stream name * source-adjust: fix sample conf * source-adjust: add spec order metadata * source-adjust: improve spec dimension doc * source-adjust: warn on custom metric cast failure * source adjust: Update source_definitions.yaml * source adjust: Update documentation url
|
Thank you, @sbjorn, for this connector.
UPDATE: Thanks |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
This adds a source connector for ingesting data from the reports API at Adjust. We stitched this together internally at Choose to get daily reports ingested into our DWH.
How
The Adjust reports API is queried to ingest desired metrics/dimensions.
Pre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampledocs/integrations/README.mdairbyte-integrations/builds.mdAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing/publishcommand described hereTests
Unit
Integration
$ ./gradlew :airbyte-integrations:connectors:source-adjust:integrationTest
Building all of Airbyte.
/airbyte/airbyte-integrations/connectors
Type-safe dependency accessors is an incubating feature.
source_adjust/init.py 2 0 100%
source_adjust/model.py 116 6 95%
source_adjust/source.py 106 35 67%
TOTAL 224 41 82%
WARNING: Support for the legacy ~/.dockercfg configuration file and file-format is deprecated and will be removed in an upcoming release
WARNING: Support for the legacy ~/.dockercfg configuration file and file-format is deprecated and will be removed in an upcoming release
checkin catalogs #1 [internal] load build definition from Dockerfile
checkin catalogs #1 sha256:e426b7f5e3e429e36067444c02bd0935c767a72afccf8d8152c534c03665ce6d
checkin catalogs #1 transferring dockerfile: 1.09kB done
checkin catalogs #1 DONE 0.0s
#2 [internal] load .dockerignore
#2 sha256:9ccb735854447cfd73f922fc3f7fa268e4d84edd8941642b06d9774ceaf3fb21
#2 transferring context: 97B done
#2 DONE 0.0s
#3 [internal] load metadata for docker.io/library/python:3.9.11-alpine3.15
#3 sha256:615609793cad3a170d91fb8a37323caa348ca31e0a749e824da4c8c1d8da2aa4
#3 DONE 0.0s
#4 [base 1/1] FROM docker.io/library/python:3.9.11-alpine3.15
#4 sha256:b26982ea68e566f4418f23b00585fceb8074e00082fc1e7cf2b85346f39804e3
#4 DONE 0.0s
#7 [internal] load build context
#7 sha256:562c9122fd7b387ad72ca3bca2609a2e9bbd7606ef2b1a232e8d9e0e5627cc43
#7 transferring context: 46.89kB done
#7 DONE 0.1s
#10 [stage-2 2/7] COPY --from=builder /install /usr/local
#10 sha256:139210fa8e9431916210eac40e667d370a2473f9310c746985e1d7b7dd3bc80a
#10 CACHED
#5 [builder 1/4] WORKDIR /airbyte/integration_code
#5 sha256:519943ee918f3d1ec01a319ff0276a627a319f59b692954112b24434e9387dff
#5 CACHED
#11 [stage-2 3/7] COPY --from=builder /usr/share/zoneinfo/Etc/UTC /etc/localtime
#11 sha256:1c30f1233193733d8883645fcbd85e3de471311188b7d4e7931c34c8e152087b
#11 CACHED
#12 [stage-2 4/7] RUN echo "Etc/UTC" > /etc/timezone
#12 sha256:36c3f086d0368137c79a27002e2c41e1e3075b497467408ff55906ea7d712eee
#12 CACHED
#8 [builder 3/4] COPY setup.py ./
#8 sha256:c944bad2c672b79aa0d42cefa9a89019b2038e85690103f6e733b2948688dea5
#8 CACHED
#6 [builder 2/4] RUN apk --no-cache upgrade && pip install --upgrade pip && apk --no-cache add tzdata build-base
#6 sha256:780e00211f02e5b1d80f59f0bbd4172193f30cfb3904193af99be9e6bc4f9f6a
#6 CACHED
#9 [builder 4/4] RUN pip install --prefix=/install .
#9 sha256:2bfd9f93afeaf5ef693718fec8d43074db51bbe80b2d36cb83885fac6992da63
#9 CACHED
#13 [stage-2 5/7] RUN apk --no-cache add bash
#13 sha256:b8ecf3fe906d22b05e6c5c3a467ad9d926b8eb6b73c42131add9a697ef6f7add
#13 CACHED
#14 [stage-2 6/7] COPY main.py ./
#14 sha256:48426654b68916c815255881f0290da826ff94e04a06875e6f366929564c6f41
#14 CACHED
#15 [stage-2 7/7] COPY source_adjust ./source_adjust
#15 sha256:4791ff0f6b2cf9d869bd9ae6716d1a23d0cf7b7f7c48c2a770e7094dc3acad31
#15 CACHED
#16 exporting to image
#16 sha256:e8c613e07b0b7ff33893b694f7759a10d42e180f2b4dc349fb57dc6b71dcab00
#16 exporting layers done
#16 writing image sha256:043d58f347df1f73ba2b69f09564562eb9c6568ad920c1b15b8aff57b79a700d done
#16 naming to docker.io/airbyte/source-adjust:dev done
#16 DONE 0.1s
#2 [internal] load .dockerignore
#2 sha256:e427b2599ab24d2575b3d21668a7acff91b11aeb0cb1dbb80dfd19f621b3455d
#2 transferring context: 100B done
#2 DONE 0.0s
#3 [internal] load metadata for docker.io/library/python:3.9.11-alpine3.15
#3 sha256:615609793cad3a170d91fb8a37323caa348ca31e0a749e824da4c8c1d8da2aa4
#3 DONE 0.0s
#4 [base 1/1] FROM docker.io/library/python:3.9.11-alpine3.15
#4 sha256:b26982ea68e566f4418f23b00585fceb8074e00082fc1e7cf2b85346f39804e3
#4 DONE 0.0s
#8 [internal] load build context
#8 sha256:23e8cb126c397b411a35499a0b912927f754ddfb29f3ab7ec52768d0c2e10af4
#8 transferring context: 222.57kB done
#8 DONE 0.0s
#10 [builder 4/5] RUN python setup.py egg_info
#10 sha256:5a0d196da5eddf7e73496ff7bf22227d807390f4c66708fb94843b5781548fa6
#10 CACHED
#13 [stage-2 3/8] COPY --from=builder /usr/share/zoneinfo/Etc/UTC /etc/localtime
#13 sha256:64296d902288275de4bac9b20e42d6172504d622cc70e2af82e5904453367e3a
#13 CACHED
#11 [builder 5/5] RUN pip install --prefix=/install -r *.egg-info/requires.txt
#11 sha256:75045d7aac1a839f9737b84da12ad8adcabb6c2fb96dc74d5e181421ea2db64f
#11 CACHED
#5 [stage-2 1/8] WORKDIR /airbyte/source_acceptance_test
#5 sha256:677495ad6746870e5ae1f2b6082de383d8be7121d20f7274b70ec719a6809222
#5 CACHED
#7 [builder 2/5] RUN apk --no-cache upgrade && pip install --upgrade pip && apk --no-cache add tzdata build-base
#7 sha256:780e00211f02e5b1d80f59f0bbd4172193f30cfb3904193af99be9e6bc4f9f6a
#7 CACHED
#15 [stage-2 5/8] RUN apk --no-cache add bash
#15 sha256:9a7e024e1575c9ca45893391d60ca7172783ecc0073f1aaf849e80fe521ada59
#15 CACHED
#6 [builder 1/5] WORKDIR /airbyte/integration_code
#6 sha256:519943ee918f3d1ec01a319ff0276a627a319f59b692954112b24434e9387dff
#6 CACHED
#16 [stage-2 6/8] COPY pytest.ini setup.py ./
#16 sha256:8f34f21e92feb94ae8ade18ebe5edfb784ebe3f3bb646a5d8b0a82359276b930
#16 CACHED
#17 [stage-2 7/8] COPY source_acceptance_test ./source_acceptance_test
#17 sha256:a78124cadc7ea311df238d7a21fe7099c0aeb993f01dcca2ffbf1086c38f2ccb
#17 CACHED
#14 [stage-2 4/8] RUN echo "Etc/UTC" > /etc/timezone
#14 sha256:22d9340d519d1c79b19b4a39ec856ee57a880904ac813f4b8e25505e8c8aafb6
#14 CACHED
#9 [builder 3/5] COPY setup.py ./
#9 sha256:e56b5d5b26ab823103b35ea16930570c9a4a9c17b688c86026263585d284c4c5
#9 CACHED
#12 [stage-2 2/8] COPY --from=builder /install /usr/local
#12 sha256:156d04127e8940cceba921b0e1c72344278b0dc9358b23b5bbfe9e6246978d52
#12 CACHED
#18 [stage-2 8/8] RUN pip install .
#18 sha256:b4743bfc6c3026cd35990866f468183054db969e2fbb2ecc01f8d93bf394c368
#18 CACHED
#19 exporting to image
#19 sha256:e8c613e07b0b7ff33893b694f7759a10d42e180f2b4dc349fb57dc6b71dcab00
#19 exporting layers done
#19 writing image sha256:4ddcd058160f47a2a29daeba03a1e64204d0a0a95109d39f10a78870072aded3 done
#19 naming to docker.io/airbyte/source-acceptance-test:dev done
#19 DONE 0.1s
test_core.py ..........s...........s.. [ 86%]
test_full_refresh.py . [ 89%]
test_incremental.py ... [100%]
=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:51: The previous connector image could not be retrieved.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:229: The previous connector image could not be retrieved.
================== 27 passed, 2 skipped in 137.97s (0:02:17) ===================
Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
See https://docs.gradle.org/7.4/userguide/command_line_interface.html#sec:command_line_warnings
Execution optimizations have been disabled for 1 invalid unit(s) of work during this build to ensure correctness.
Please consult deprecation warnings for more details.
BUILD SUCCESSFUL in 8m 24s
48 actionable tasks: 31 executed, 7 from cache, 10 up-to-date
Acceptance