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 Pipedrive: handle missed cursor_field #24282

Merged
merged 8 commits into from Mar 21, 2023

Conversation

grubberr
Copy link
Contributor

@grubberr grubberr commented Mar 21, 2023

What

Issue: #16376
Issue: https://github.com/airbytehq/alpha-beta-issues/issues/1002
Issue: #23687

  • Increase Users.page_size = 500 for Users stream. it seems pagination does not work for Users stream.
    We cannot reproduce the issue on airbyte testing data. Temporary solution until the more better solution will be found.
    Fix issue: Source Pipedrive: stuck reading Users records #23687

  • Skip re-evaluation state if cursor_field is missed in record.

Updating a connector

Community member or Airbyter

  • Grant edit access to maintainers (instructions)

  • Secrets in the connector's spec are annotated with airbyte_secret

  • 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

  • Connector version has been incremented

  • Documentation updated

    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. 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

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@grubberr
Copy link
Contributor Author

/test connector=connectors/source-pipedrive

2 similar comments
@grubberr
Copy link
Contributor Author

/test connector=connectors/source-pipedrive

@grubberr
Copy link
Contributor Author

/test connector=connectors/source-pipedrive

@grubberr
Copy link
Contributor Author

grubberr commented Mar 21, 2023

/test connector=connectors/source-pipedrive

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

Name                           Stmts   Miss  Cover
--------------------------------------------------
source_pipedrive/auth.py           8      0   100%
source_pipedrive/__init__.py       2      0   100%
source_pipedrive/source.py        38      2    95%
source_pipedrive/streams.py      102     15    85%
--------------------------------------------------
TOTAL                            150     17    89%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:509: The previous and actual discovered catalogs are identical.
================== 36 passed, 2 skipped in 133.30s (0:02:13) ===================

Comment on lines 93 to 99
updated_state = latest_record.get(self.cursor_field)
if updated_state:
stream_state_value = current_stream_state.get(self.cursor_field)
if stream_state_value:
updated_state = max(updated_state, stream_state_value)
current_stream_state[self.cursor_field] = updated_state
return current_stream_state
Copy link
Collaborator

Choose a reason for hiding this comment

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

According to spec.json replication_start_date is required and all cursor fields is date fields. So, we can used it as default cursor value. Also please check request_params method (line 62).

Suggested change
updated_state = latest_record.get(self.cursor_field)
if updated_state:
stream_state_value = current_stream_state.get(self.cursor_field)
if stream_state_value:
updated_state = max(updated_state, stream_state_value)
current_stream_state[self.cursor_field] = updated_state
return current_stream_state
updated_state = latest_record.get(self.cursor_field, self._replication_start_date)
stream_state_value = current_stream_state.get(self.cursor_field, self._replication_start_date)
current_stream_state[self.cursor_field] = max(updated_state, stream_state_value)
return current_stream_state

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lazebnyi
I have switched replication_start_date as default if cursor field is missed in record

about line 62
there is if condition because in "full refresh" streams self._replication_start_date still can be None.

Copy link
Collaborator

Choose a reason for hiding this comment

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

replication_start_date is required, so I don't see any situations where it can be empty during sync.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@grubberr
Copy link
Contributor Author

grubberr commented Mar 21, 2023

/test connector=connectors/source-pipedrive

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

Name                           Stmts   Miss  Cover
--------------------------------------------------
source_pipedrive/auth.py           8      0   100%
source_pipedrive/__init__.py       2      0   100%
source_pipedrive/source.py        38      2    95%
source_pipedrive/streams.py      101     15    85%
--------------------------------------------------
TOTAL                            149     17    89%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:509: The previous and actual discovered catalogs are identical.
================== 36 passed, 2 skipped in 142.37s (0:02:22) ===================

@grubberr grubberr requested a review from lazebnyi March 21, 2023 16:05
@grubberr grubberr requested a review from lazebnyi March 21, 2023 18:45
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Mar 21, 2023
@grubberr
Copy link
Contributor Author

grubberr commented Mar 21, 2023

/test connector=connectors/source-pipedrive

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

Name                           Stmts   Miss  Cover
--------------------------------------------------
source_pipedrive/auth.py           8      0   100%
source_pipedrive/__init__.py       2      0   100%
source_pipedrive/source.py        38      2    95%
source_pipedrive/streams.py       98     15    85%
--------------------------------------------------
TOTAL                            146     17    88%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:509: The previous and actual discovered catalogs are identical.
================== 36 passed, 2 skipped in 141.14s (0:02:21) ===================

@grubberr
Copy link
Contributor Author

grubberr commented Mar 21, 2023

/publish connector=connectors/source-pipedrive

🕑 Publishing the following connectors:
connectors/source-pipedrive
https://github.com/airbytehq/airbyte/actions/runs/4483345849


Connector Did it publish? Were definitions generated?
connectors/source-pipedrive

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@grubberr grubberr merged commit 164d1c1 into master Mar 21, 2023
20 of 21 checks passed
@grubberr grubberr deleted the grubberr/16376-source-pipedrive branch March 21, 2023 20:49
erohmensing pushed a commit that referenced this pull request Mar 22, 2023
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
erohmensing pushed a commit that referenced this pull request Mar 22, 2023
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
am6010 pushed a commit to rudderlabs/airbyte that referenced this pull request Mar 27, 2023
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
(cherry picked from commit 164d1c1)
am6010 added a commit to rudderlabs/airbyte that referenced this pull request Mar 27, 2023
* Source Pipedrive: handle missed cursor_field (airbytehq#24282)

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
(cherry picked from commit 164d1c1)

* chore: remove oauth for pipedrive

---------

Co-authored-by: Serhii Chvaliuk <grubberr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/pipedrive
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants