Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source Amplitude: Add undeclared columns to spec #24480

Closed
erohmensing opened this issue Mar 24, 2023 · 1 comment · Fixed by #25842
Closed

Source Amplitude: Add undeclared columns to spec #24480

erohmensing opened this issue Mar 24, 2023 · 1 comment · Fixed by #25842
Assignees

Comments

@erohmensing
Copy link
Contributor

What

In this PR, BasicReadTest.test_read test in the Connector Acceptance Tests was updated to fail if the connector produces
stream records which contain columns that haven't been declared in the spec.

Amplitude currently fails the updated test. Its acceptance-test-config.yml was edited with the
fail_on_extra_columns: false parameter in order to avoid this change from making the connector fail CAT.

We want to add the undeclared columns to the spec.

How this will help:

  • Column selection is currently blocked for connectors that don't declare all columns that they pass. This
    is because turning on column selection will stop these undeclared columns from being sent, when they were
    previously being sent. Doing this allows us to enable column selection for Amplitude!
  • Users will better understand the shape of the data they'll receive, since the columns will match the spec.

How

The following descriptions of streams that pass undeclared columns come from results of the failed connector acceptance test:

columns.txt:./source-amplitude.txt-85-The events stream has the following schema errors:
columns.txt:./source-amplitude.txt:86:Additional properties are not allowed ('$insert_key', 'data_type', 'plan', 'source_id', 'partner_id', 'global_user_properties' were unexpected)

  1. Add the missing properties indicated by the Additional properties are not allowed ('<column>', 'column' were unexpected) logs to the connector's spec.
  2. Remove fail_on_extra_columns: false from the acceptance-test-config.yml file.
  3. Commit changes to spec and acceptance-test-config.yml and open a PR.
  4. Run tests on the connector, either automatically via CI or manually via the /test command
  5. Profit!

Definition of done: Amplitude passes CAT without declaring fail_on_extra_columns: false. If the
API starts sending over extra columns in the future, we will catch and fix them as part of the #connector-health
movement.

@evantahler
Copy link
Contributor

When this issue is picked up, please also consult our DataDog Dashboard for Record Unexpected Fields for this connector. The API might be returning even more record properties that need to be added to the catalog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants