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

SAT: cursor_paths should support custom nested and absolute paths #4552

Merged
merged 11 commits into from
Jul 5, 2021
Merged

SAT: cursor_paths should support custom nested and absolute paths #4552

merged 11 commits into from
Jul 5, 2021

Conversation

keu
Copy link
Contributor

@keu keu commented Jul 5, 2021 •
edited

What

These changes allow to specify cursor path in state file as following:

      cursor_paths:
        subscription_changes: ["account_id_1234", "timestamp"]
        email_events: ["bookmarks", "email_events", "timestamp"]

this covers custom states that singer produces and states for nested streams when we include parent stream ID in the path.

How

Describe the solution

Recommended reading order

  1. x.java
  2. y.python

Pre-merge Checklist

Expand the checklist which is relevant for this PR.

sherifnada
sherifnada approved these changes Jul 5, 2021
View reviewed changes
...integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py Outdated
return pendulum.parse(value)
return value

def parse(self, record, path=None) -> Any:
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add typehints to this method?

...integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py Outdated
class Field:
"""Field class to represent cursor/pk fields"""

def __init__(self, schema, path):
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add typehints?

...integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py Outdated

import pendulum


class Field:
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this class!

maybe a better name for this class would be CatalogField? Just trying to avoid conflict with Field which is already used by pydantic

...e-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py Show resolved Hide resolved
keu and others added 2 commits July 5, 2021 22:20
@keu @sherifnada
Update airbyte-integrations/bases/source-acceptance-test/source_accep…
e84e779 
…tance_test/tests/test_incremental.py

Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
@eugene-kulak
address comments from @sherifnada
3adbbab
@keu
Copy link
Contributor Author

keu commented Jul 5, 2021 •
edited by github-actions bot

/publish connector=bases/source-acceptance-test

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

@keu keu added the Acceptance Tests https://docs.google.com/document/d/1pFLR8eLP5VOj6vDtWe69KZL0aWWHEA6Vkbod6NYJon4/edit# label Jul 5, 2021
@keu
Copy link
Contributor Author

keu commented Jul 5, 2021

bumping version to 0.1.8 because 0.1.7 has being published but not merged

@keu
Copy link
Contributor Author

keu commented Jul 5, 2021 •
edited by github-actions bot

/publish connector=bases/source-acceptance-test

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

@keu keu merged commit f4ceebe into master Jul 5, 2021
@keu keu deleted the keu/sat-nested-cursor-path branch July 5, 2021 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sherifnada sherifnada sherifnada approved these changes

@htrueman htrueman Awaiting requested review from htrueman

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

Assignees
No one assigned
Labels
Acceptance Tests https://docs.google.com/document/d/1pFLR8eLP5VOj6vDtWe69KZL0aWWHEA6Vkbod6NYJon4/edit#
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@keu @sherifnada @eugene-kulak