🐛 Source Amplitude: Fixed JSON Validator `date-time` validation #13373

bazarnov · 2022-06-01T11:37:38Z

What

Resolving: #13075

How

added _get_date_time_items_from_schema method to get all date-time formated items from schema
added _date_time_to_rfc3339 method to transform record date-time entities to RFC3339 format
added unit_tests to cover the case

🚨 User Impact 🚨

No impact expected.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

Updating a connector

Community member or Airbyter

Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and 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.
Code reviews completed
Documentation updated
- Connector's README.md
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

bazarnov · 2022-06-01T11:41:21Z

@pedroslopez please check this PR according to this discussion: #13075

bazarnov · 2022-06-01T12:47:16Z

/test connector=connectors/source-amplitude

🕑 connectors/source-amplitude https://github.com/airbytehq/airbyte/actions/runs/2421665113
✅ connectors/source-amplitude https://github.com/airbytehq/airbyte/actions/runs/2421665113
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        77      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/tests/test_incremental.py       121     25    79%
source_acceptance_test/utils/common.py                  80     17    79%
source_acceptance_test/tests/test_core.py              294    106    64%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
------------------------------------------------------------------------
TOTAL                                                  960    246    74%
Name                           Stmts   Miss  Cover
--------------------------------------------------
source_amplitude/source.py        24      0   100%
source_amplitude/__init__.py       2      0   100%
source_amplitude/errors.py        12      1    92%
source_amplitude/api.py          149     18    88%
--------------------------------------------------
TOTAL                            187     19    90%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_incremental.py:23: `future_state_path` not specified, skipping
================== 23 passed, 1 skipped in 152.11s (0:02:32) ===================

pedroslopez

I think the right approach here is to follow what Hubspot did in #13159 and instead fix the connector to output date-times in RFC3339 format.

From my comment in #13075:

We are using JSONSchema to specify types, and JSONSchema specifically states that date-times should be represented RFC3339. As such, I think the right solution is to have connectors output date-times in RFC3339, rather than simply remove "format": "date-time" from the stream schemas.

…ce-amplitude-fix-date-time-validation

bazarnov · 2022-06-02T11:39:14Z

/test connector=connectors/source-amplitude

🕑 connectors/source-amplitude https://github.com/airbytehq/airbyte/actions/runs/2427963867
✅ connectors/source-amplitude https://github.com/airbytehq/airbyte/actions/runs/2427963867
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        77      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/tests/test_incremental.py       121     25    79%
source_acceptance_test/utils/common.py                  80     17    79%
source_acceptance_test/tests/test_core.py              294    106    64%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
------------------------------------------------------------------------
TOTAL                                                  960    246    74%
Name                           Stmts   Miss  Cover
--------------------------------------------------
source_amplitude/source.py        24      0   100%
source_amplitude/__init__.py       2      0   100%
source_amplitude/errors.py        12      1    92%
source_amplitude/api.py          165     18    89%
--------------------------------------------------
TOTAL                            203     19    91%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_incremental.py:23: `future_state_path` not specified, skipping
================== 23 passed, 1 skipped in 123.27s (0:02:03) ===================

codecov · 2022-06-02T11:43:57Z

Codecov Report

❗ No coverage uploaded for pull request base (master@d79b319). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 5a0ce03 differs from pull request most recent head 98f3d2d. Consider uploading reports for the commit 98f3d2d to get more accurate results

@@            Coverage Diff            @@
##             master   airbytehq/airbyte#13373   +/-   ##
=========================================
  Coverage          ?   92.03%           
=========================================
  Files             ?        4           
  Lines             ?      201           
  Branches          ?        0           
=========================================
  Hits              ?      185           
  Misses            ?       16           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d79b319...98f3d2d. Read the comment docs.

airbyte-integrations/connectors/source-amplitude/source_amplitude/api.py

bazarnov · 2022-06-03T11:07:56Z

/test connector=connectors/source-amplitude

🕑 connectors/source-amplitude https://github.com/airbytehq/airbyte/actions/runs/2434214434
✅ connectors/source-amplitude https://github.com/airbytehq/airbyte/actions/runs/2434214434
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        77      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/tests/test_incremental.py       121     25    79%
source_acceptance_test/utils/common.py                  80     17    79%
source_acceptance_test/tests/test_core.py              294    106    64%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
------------------------------------------------------------------------
TOTAL                                                  960    246    74%
Name                           Stmts   Miss  Cover
--------------------------------------------------
source_amplitude/source.py        24      0   100%
source_amplitude/__init__.py       2      0   100%
source_amplitude/errors.py        12      1    92%
source_amplitude/api.py          163     15    91%
--------------------------------------------------
TOTAL                            201     16    92%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_incremental.py:23: `future_state_path` not specified, skipping
================== 23 passed, 1 skipped in 144.58s (0:02:24) ===================

pedroslopez

Nice - verified that timestamps from the amplitude export api are actually UTC according to docs, so this should be good:

The Export API returns events timestamped in UTC

https://developers.amplitude.com/docs/export-api

bazarnov · 2022-06-03T14:22:31Z

/publish connector=connectors/source-amplitude

🕑 connectors/source-amplitude https://github.com/airbytehq/airbyte/actions/runs/2435186994
🚀 Successfully published connectors/source-amplitude
🚀 Auto-bumped version for connectors/source-amplitude
✅ connectors/source-amplitude https://github.com/airbytehq/airbyte/actions/runs/2435186994

* master: (142 commits) Highlight removed and added streams in Connection form (airbytehq#13392) 🐛 Source Amplitude: Fixed JSON Validator `date-time` validation (airbytehq#13373) 🐛 Source Mixpanel: publish v0.1.17 (airbytehq#13450) Fixed reverted PR: Fix cancel button when it doesn't provide feedback to the user + UX improvements (airbytehq#13388) 🎉 Source Freshdesk: Added new streams (airbytehq#13332) Prepare YamlSeedConfigPersistence for dependency injection (airbytehq#13384) helm chart: Support nodeSelector, tolerations and affinity on the booloader pod (airbytehq#11467) airbyte-api: add jackson model annotations to remove null values from responses (airbytehq#13370) Change stage to `beta` (airbytehq#13422) 🐛 Source Google Sheets: Retry on server errors (airbytehq#13446) Improve kube deploy process. (airbytehq#13397) Helm chart dependencies fix (airbytehq#13432) 🐛 Source HubSpot: Transform `contact_lists` data to comply with schema (airbytehq#13218) airbytehq#11758: Source Google Ads to GA (airbytehq#13441) Add more pr actions to tag pull requests (airbytehq#13437) Source Google Ads: drop schema field that filters out the data from stream (airbytehq#13423) Updates error view with new design (airbytehq#13197) Source MSSQL: correct enum Standard method (airbytehq#13419) Update postgres doc about cdc publication (airbytehq#13433) run source acceptance tests against image built from branch (airbytehq#13401) ...

bazarnov added 2 commits June 1, 2022 14:30

fixed

de3603b

updated readme

1498faa

bazarnov self-assigned this Jun 1, 2022

github-actions bot added the area/connectors Connector related issues label Jun 1, 2022

updated changelog

33022df

bazarnov linked an issue Jun 1, 2022 that may be closed by this pull request

🐛 Source Amplitude: Failed to apply RFC3339 pattern on date-time string #13075

Closed

github-actions bot added the area/documentation Improvements or additions to documentation label Jun 1, 2022

bazarnov requested a review from pedroslopez June 1, 2022 11:40

airbytehq deleted a comment from codecov bot Jun 1, 2022

pedroslopez reviewed Jun 1, 2022

View reviewed changes

bazarnov added 3 commits June 2, 2022 12:28

reverted schema change

eb90e20

Merge remote-tracking branch 'origin/master' into bazarnov/13075-sour…

4c69d04

…ce-amplitude-fix-date-time-validation

fixed

aedcfed

airbytehq deleted a comment from codecov bot Jun 2, 2022

bazarnov mentioned this pull request Jun 2, 2022

🐛 Source Amplitude: Failed to apply RFC3339 pattern on date-time string #13075

Closed

bazarnov requested a review from pedroslopez June 2, 2022 11:47

sherifnada reviewed Jun 2, 2022

View reviewed changes

airbyte-integrations/connectors/source-amplitude/source_amplitude/api.py Show resolved Hide resolved

bleonard added the connectors/source/amplitude label Jun 2, 2022

bazarnov requested a review from sherifnada June 2, 2022 21:25

sherifnada reviewed Jun 3, 2022

View reviewed changes

airbyte-integrations/connectors/source-amplitude/source_amplitude/api.py Show resolved Hide resolved

airbyte-integrations/connectors/source-amplitude/source_amplitude/api.py Outdated Show resolved Hide resolved

updated after review

5a0ce03

bazarnov requested a review from sherifnada June 3, 2022 11:42

pedroslopez approved these changes Jun 3, 2022

View reviewed changes

auto-bump connector version

98f3d2d

octavia-squidington-iii temporarily deployed to more-secrets June 3, 2022 14:41 Inactive

bazarnov merged commit 94dc191 into master Jun 3, 2022

bazarnov deleted the bazarnov/13075-source-amplitude-fix-date-time-validation branch June 3, 2022 14:41

octavia-squidington-iii mentioned this pull request Jun 3, 2022

Bump Airbyte version from 0.39.8-alpha to 0.39.9-alpha #13454

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Source Amplitude: Fixed JSON Validator `date-time` validation #13373

🐛 Source Amplitude: Fixed JSON Validator `date-time` validation #13373

bazarnov commented Jun 1, 2022 •

edited

Loading

bazarnov commented Jun 1, 2022

bazarnov commented Jun 1, 2022 •

edited by github-actions bot

Loading

pedroslopez left a comment

bazarnov commented Jun 2, 2022 •

edited by github-actions bot

Loading

codecov bot commented Jun 2, 2022 •

edited

Loading

bazarnov commented Jun 3, 2022 •

edited by github-actions bot

Loading

pedroslopez left a comment

bazarnov commented Jun 3, 2022 •

edited by github-actions bot

Loading

🐛 Source Amplitude: Fixed JSON Validator date-time validation #13373

🐛 Source Amplitude: Fixed JSON Validator date-time validation #13373

Conversation

bazarnov commented Jun 1, 2022 • edited Loading

What

How

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

bazarnov commented Jun 1, 2022

bazarnov commented Jun 1, 2022 • edited by github-actions bot Loading

Build Passed

pedroslopez left a comment

Choose a reason for hiding this comment

bazarnov commented Jun 2, 2022 • edited by github-actions bot Loading

Build Passed

codecov bot commented Jun 2, 2022 • edited Loading

Codecov Report

bazarnov commented Jun 3, 2022 • edited by github-actions bot Loading

Build Passed

pedroslopez left a comment

Choose a reason for hiding this comment

bazarnov commented Jun 3, 2022 • edited by github-actions bot Loading

🐛 Source Amplitude: Fixed JSON Validator `date-time` validation #13373

🐛 Source Amplitude: Fixed JSON Validator `date-time` validation #13373

bazarnov commented Jun 1, 2022 •

edited

Loading

bazarnov commented Jun 1, 2022 •

edited by github-actions bot

Loading

bazarnov commented Jun 2, 2022 •

edited by github-actions bot

Loading

codecov bot commented Jun 2, 2022 •

edited

Loading

bazarnov commented Jun 3, 2022 •

edited by github-actions bot

Loading

bazarnov commented Jun 3, 2022 •

edited by github-actions bot

Loading