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.

Sign up for GitHub

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

Jump to bottom

Add source_defined_primary_key validation test to test_read #4140

Merged
merged 6 commits into from Jun 25, 2021
Merged

Add source_defined_primary_key validation test to test_read #4140

merged 6 commits into from Jun 25, 2021

Conversation

htrueman
Copy link
Contributor

What

Closes #2748

How

Updated TestBasicRead.test_read test.
Added validation to check if primary key presents and not null in output records in case when source_defined_primary_key set in cofigured catalog.

Recommended reading order

  1. test_core.py

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 output spec
  • 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
  • Credentials added to Github CI if needed and not already present. instructions for injecting secrets into CI.
  • Documentation updated
    • README
    • CHANGELOG.md
    • Reference docs in the docs/integrations/ directory.
    • 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.

@htrueman htrueman requested a review from Zirochkaa June 24, 2021 12:50
@htrueman htrueman requested a review from keu June 24, 2021 14:40
keu
keu approved these changes Jun 25, 2021
View reviewed changes
Copy link
Contributor

@keu keu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgmt with small changes that we discussed offline

@htrueman
Update test_core.py source_defined_primary_key test.
1213209 
Add parse_value date type support.
@htrueman
Merge remote-tracking branch 'origin/master' into htrueman/sat-verify…
7820262 
…-pk-is-not-null

# Conflicts:
#	airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py
@htrueman
Bump docker version
f91cc24
@htrueman
Copy link
Contributor Author

htrueman commented Jun 25, 2021 •
edited by github-actions bot

/publish connector=bases/source-acceptance-test

🕑 bases/source-acceptance-test https://github.com/airbytehq/airbyte/actions/runs/971874672
✅ bases/source-acceptance-test https://github.com/airbytehq/airbyte/actions/runs/971874672

@htrueman htrueman merged commit d23b883 into master Jun 25, 2021
@htrueman htrueman deleted the htrueman/sat-verify-pk-is-not-null branch June 25, 2021 15:28
@htrueman htrueman linked an issue Aug 2, 2021 that may be closed by this pull request
SAT: Fix configured_catalog stream field fulfillment #5142
Closed
@snyk-bot snyk-bot mentioned this pull request Feb 7, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keu keu keu approved these changes

@Zirochkaa Zirochkaa Zirochkaa approved these changes

@vitaliizazmic vitaliizazmic vitaliizazmic approved these changes

@yevhenii-ldv yevhenii-ldv Awaiting requested review from yevhenii-ldv

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@htrueman @keu @Zirochkaa @vitaliizazmic