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

CDK: VCR -> requests_cache + SQLite #17990

Merged
merged 49 commits into from
Oct 19, 2022
Merged

Conversation

grubberr
Copy link
Contributor

@grubberr grubberr commented Oct 14, 2022

Signed-off-by: Sergey Chvalyuk grubberr@gmail.com

What

Replace caching method: VCR.py -> requests-cache with SQLite backend

  • VCR store cached data in one YAML file, for large stream it can consume too much memory

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@grubberr grubberr requested a review from a team October 14, 2022 09:01
@grubberr grubberr self-assigned this Oct 14, 2022
@github-actions github-actions bot added the CDK Connector Development Kit label Oct 14, 2022
@grubberr grubberr linked an issue Oct 14, 2022 that may be closed by this pull request
@evantahler
Copy link
Contributor

The rationale and test results for this PR are here => #17919

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
…fferent

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

grubberr commented Oct 16, 2022

/test connector=connectors/source-github

🕑 connectors/source-github https://github.com/airbytehq/airbyte/actions/runs/3260052875
❌ connectors/source-github https://github.com/airbytehq/airbyte/actions/runs/3260052875
🐛 https://gradle.com/s/lbkz6i74msowy

Build Failed

Test summary info:

	 =========================== short test summary info ============================
	 FAILED unit_tests/test_stream.py::test_stream_project_columns - AssertionErro...
	 FAILED unit_tests/test_stream.py::test_stream_commit_comment_reactions_incremental_read
	 �[31m================== �[31m�[1m2 failed�[0m, �[32m44 passed�[0m, �[33m151 warnings�[0m�[31m in 2.87s�[0m�[31m ==================�[0m

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

grubberr commented Oct 17, 2022

/test connector=connectors/source-github

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

Name                             Stmts   Miss  Cover
----------------------------------------------------
source_github/utils.py              14      0   100%
source_github/github_schema.py    8807      0   100%
source_github/__init__.py            2      0   100%
source_github/graphql.py           145      1    99%
source_github/streams.py           792     44    94%
source_github/source.py            104     27    74%
----------------------------------------------------
TOTAL                             9864     72    99%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          10      4    60%   15-18
	 source_acceptance_test/config.py                        83      6    93%   78-80, 84-86
	 source_acceptance_test/conftest.py                     164    164     0%   6-282
	 source_acceptance_test/plugin.py                        48     48     0%   6-104
	 source_acceptance_test/tests/test_core.py              329    111    66%   39, 50-58, 63-70, 74-75, 79-80, 164, 202-219, 228-236, 240-245, 251, 284-289, 327-334, 374-376, 379, 439-448, 477-478, 484, 487, 520-530, 543-568, 573-577
	 source_acceptance_test/tests/test_full_refresh.py       52      2    96%   34, 65
	 source_acceptance_test/tests/test_incremental.py       145     20    86%   21-23, 29-31, 36-43, 48-61, 224
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     10    87%   15-16, 24-30, 64, 67
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       112     50    55%   23-26, 32, 36, 39-67, 70-72, 75-77, 80-82, 85-87, 90-92, 95-113, 147-149
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1351    453    66%

Build Passed

Test summary info:

All Passed

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

grubberr commented Oct 17, 2022

/test connector=connectors/source-okta

🕑 connectors/source-okta https://github.com/airbytehq/airbyte/actions/runs/3263216811
❌ connectors/source-okta https://github.com/airbytehq/airbyte/actions/runs/3263216811
🐛 https://gradle.com/s/p4w442o6yw43e

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestConnection::test_check[inputs0] - AssertionError: as...
FAILED test_core.py::TestConnection::test_check[inputs1] - AssertionError: as...
FAILED test_core.py::TestConnection::test_check[inputs2] - AssertionError: as...
FAILED test_core.py::TestBasicRead::test_read[inputs0] - docker.errors.Contai...
FAILED test_core.py::TestBasicRead::test_read[inputs1] - docker.errors.Contai...
FAILED test_full_refresh.py::TestFullRefresh::test_sequential_reads[inputs0]
FAILED test_incremental.py::TestIncremental::test_two_sequential_reads[inputs0]
FAILED test_incremental.py::TestIncremental::test_read_sequential_slices[inputs0]
FAILED test_incremental.py::TestIncremental::test_state_with_abnormally_large_values[inputs0]
======================== 9 failed, 24 passed in 26.95s =========================

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

grubberr commented Oct 19, 2022

/publish-cdk dry-run=true

🕑 https://github.com/airbytehq/airbyte/actions/runs/3283226839
https://github.com/airbytehq/airbyte/actions/runs/3283226839

@grubberr
Copy link
Contributor Author

grubberr commented Oct 19, 2022

/publish-cdk dry-run=false

🕑 https://github.com/airbytehq/airbyte/actions/runs/3283290983
https://github.com/airbytehq/airbyte/actions/runs/3283290983

@grubberr grubberr merged commit 258b23c into master Oct 19, 2022
@grubberr grubberr deleted the grubberr/17919-airbyte_cdk branch October 19, 2022 16:31
@evantahler
Copy link
Contributor

evantahler commented Oct 19, 2022

Woo! In the very near future, we should publish a new version of the GitHub source using this new CDK version so we can measure the speed up (and so the source will work).

Cc @YowanR

@YowanR
Copy link
Contributor

YowanR commented Oct 19, 2022

Pardon my ignorance here, but what does it actually entail to release a source with the new CDK version?

@grubberr
Copy link
Contributor Author

@evantahler #18213

@YowanR in this PR we updated version of airbtye_cdk 0.1.104 -> 0.2.0
caching mechanism was changed - on cdk level

After this PR merged we need to re-release all connectors which use caching to catch-up this new changes

@grubberr grubberr mentioned this pull request Oct 22, 2022
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CDK Connector Development Kit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API Connector Memory leaks
5 participants