🎉 New source: Dixa

[olivermeyer]

What

In this PR I implement a source-dixa connector.

How

The connector supports both full refresh and incremental syncs. It uses the updated_at field (millisecond-precision timestamp) from the response as a cursor.

Pre-merge Checklist

Expand the checklist which is relevant for this PR.

Connector checklist

• Issue acceptance criteria met
• PR name follows PR naming conventions
• Secrets are annotated with airbyte_secret in the connector's spec
• Credentials added to Github CI if needed and not already present. instructions for injecting secrets into CI.
• 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
• Documentation updated
  • README.md
  • docs/SUMMARY.md if it's a new connector
  • Created or updated reference docs in docs/integrations/<source or destination>/<name>.
  • Changelog in the appropriate page in docs/integrations/.... See changelog example
  • docs/integrations/README.md contains a reference to the new connector
  • Build status added to build page
• 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

Connector Generator checklist

• Issue acceptance criteria met
• PR name follows PR naming conventions
• If adding a new generator, add it to the list of scaffold modules being tested
• The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
• Documentation which references the generator is updated as needed.

[sherifnada]

@olivermeyer great stuff!

[sherifnada]

Would it be wise to make this a little smaller e.g: a week or less? Since the endpoint doesn't have any pagination strategy, we could end up making gigantic requests otherwise

[olivermeyer]

Good point. The reason I pushed it to the highest possible value was that the rate limit is quite low (10 requests per 60 seconds). With batches of 7 days, the connector would spend around 5 minutes sleeping when importing a whole year. With batches of one day, that number jumps to >30 minutes... Not ideal.

This wouldn't be that big of a problem if incremental is an option, but still worth considering. What do you think of making the batch size a parameter of the connector?

[sherifnada]

making it a param is a great idea. It can be an optional parameter with a default value of 31.

[olivermeyer]

Acceptance tests output:

$ python -m pytest -p integration_tests.acceptance
Test session starts (platform: darwin, Python 3.7.9, pytest 6.2.4, pytest-sugar 0.9.4)
cachedir: .pytest_cache
rootdir: /Users/Omeyer/code/perso/airbyte, configfile: pytest.ini
plugins: sugar-0.9.4, timeout-1.4.2
collecting ...
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestSpec.test_spec[inputs0] ✓                                                                   14% █▌
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestConnection.test_check[inputs0] ✓                                                            29% ██▉
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestConnection.test_check[inputs1] ✓                                                            43% ████▍
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestDiscovery.test_discover[inputs0] ✓                                                          57% █████▊
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestBasicRead.test_read[inputs0] ✓                                                              71% ███████▎
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py::TestIncremental.test_two_sequential_reads[inputs0] ✓                                     86% ████████▋
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py::TestIncremental.test_state_with_abnormally_large_values[inputs0] s                      100% ██████████
========================================================================================= short test summary info ==========================================================================================
SKIPPED [1] ../../bases/source-acceptance-test/source_acceptance_test/plugin.py:76: Skipping TestFullRefresh.test_sequential_reads because not found in the config
SKIPPED [1] ../../bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py:41: `future_state_path` not specified, skipping

Results (10.48s):
       6 passed
       1 skipped

[sherifnada]

@olivermeyer Looks great! I love the thorough unit test coverage 💪🏼

I made some usability/readability improvement suggestions, but they're not required if you don't have the bandwidth. LMK and we can move forward!

[sherifnada]

would it be more user friendly to make this a human readable date? e.g: YYYY-MM-DD?

You can then easily use pendulum.parse(date).timestamp() to make it into a timestamp

[olivermeyer]

It definitely makes sense to take a start_date as argument. I'll convert it to ms-timestamp internally.

[sherifnada]

Just a suggestion: I would recommend making this a date string like ISO8601 or just YYYY-MM-DD to make it easier for humans to work with. I think that would mostly obviate the need for a timestamp.

[olivermeyer]

The API can take two parameter formats: millisecond-timestamps or YYYY-MM-DD strings as you described. Of the two, I'd rather pass it timestamps as they're a lot more granular, and I'd rather avoid loading a whole day's worth of data if I can load just a couple of hours. What do you think?

[sherifnada]

if the user ever updates start_timestamp to be higher than the state then we should pick it up over the state. It might be doable via something like:

stream_state = stream_state or {}
updated_after = max(stream_state.get(cursor_field, self.start_timestamp), self.start_timestamp)

[olivermeyer]

Makes sense 👍

[sherifnada]

would this block be simpler if we do something like:

slices = []
updated_after = ...
while updated_after < self.end_timestamp:
   updated_before = min(updated_after + batch, etc....)
   slices.append({'before':...})
   updated_after = updated_before

return slices

This way we're only appending once inside the while loop

Just a small readability improvement probably

[olivermeyer]

You're right - I tried to do something like that at first but couldn't get it to work as expected. It seems fine now, and tests are passing so 🤷

[olivermeyer]

Acceptance tests output after latest commit:

$ python -m pytest -p integration_tests.acceptance
Test session starts (platform: darwin, Python 3.7.9, pytest 6.2.4, pytest-sugar 0.9.4)
cachedir: .pytest_cache
rootdir: /Users/Omeyer/code/perso/airbyte, configfile: pytest.ini
plugins: sugar-0.9.4, timeout-1.4.2
collecting ...
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestSpec.test_spec[inputs0] ✓                                                                   14% █▌
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestConnection.test_check[inputs0] ✓                                                            29% ██▉
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestConnection.test_check[inputs1] ✓                                                            43% ████▍
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestDiscovery.test_discover[inputs0] ✓                                                          57% █████▊
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestBasicRead.test_read[inputs0] ✓                                                              71% ███████▎
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py::TestIncremental.test_two_sequential_reads[inputs0] ✓                                     86% ████████▋
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py::TestIncremental.test_state_with_abnormally_large_values[inputs0] s                      100% ██████████
========================================================================================= short test summary info ==========================================================================================
SKIPPED [1] ../../bases/source-acceptance-test/source_acceptance_test/plugin.py:76: Skipping TestFullRefresh.test_sequential_reads because not found in the config
SKIPPED [1] ../../bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py:41: `future_state_path` not specified, skipping

Results (42.06s):
       6 passed
       1 skipped```