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

🎉Source Monday: migration to low-code #19586

Merged
merged 5 commits into from
Dec 13, 2022
Merged

🎉Source Monday: migration to low-code #19586

merged 5 commits into from
Dec 13, 2022

Conversation

roman-yermilov-gl
Copy link
Contributor

What

Migrate to low-code

@roman-yermilov-gl roman-yermilov-gl self-assigned this Nov 18, 2022
davydov-d
davydov-d requested changes Nov 21, 2022
View reviewed changes
airbyte-integrations/connectors/source-monday/acceptance-test-config.yml Show resolved Hide resolved
airbyte-integrations/connectors/source-monday/acceptance-test-config.yml Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-monday/acceptance-test-config.yml Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-monday/source_monday/monday.yaml Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-monday/source_monday/graphql_request_options_provider.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-monday/source_monday/pagination_workaround.py Outdated Show resolved Hide resolved
...te-integrations/connectors/source-monday/unit_tests/test_graphql_request_options_provider.py Outdated Show resolved Hide resolved
...te-integrations/connectors/source-monday/unit_tests/test_graphql_request_options_provider.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-monday/source_monday/pagination_workaround.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-monday/unit_tests/test_pagination_workaround.py Outdated Show resolved Hide resolved
@octavia-squidington-iv octavia-squidington-iv added the area/documentation Improvements or additions to documentation label Nov 25, 2022
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets November 25, 2022 12:46 Inactive
davydov-d
davydov-d reviewed Nov 28, 2022
View reviewed changes
airbyte-integrations/connectors/source-monday/source_monday/graphql_request_options_provider.py Outdated Show resolved Hide resolved
...te-integrations/connectors/source-monday/unit_tests/test_graphql_request_options_provider.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-monday/unit_tests/test_pagination_workaround.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-monday/source_monday/graphql_request_options_provider.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-monday/source_monday/graphql_request_options_provider.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-monday/source_monday/graphql_request_options_provider.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-monday/source_monday/graphql_request_options_provider.py Show resolved Hide resolved
airbyte-integrations/connectors/source-monday/source_monday/graphql_request_options_provider.py Show resolved Hide resolved
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets November 28, 2022 13:48 Inactive
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets November 28, 2022 14:14 Inactive
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets November 28, 2022 18:48 Inactive
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets November 30, 2022 18:10 Inactive
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets November 30, 2022 18:33 Inactive
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets November 30, 2022 18:36 Inactive
@lazebnyi lazebnyi changed the title Source Monday: migration to low-code 🎉Source Monday: migration to low-code Dec 6, 2022
girarda
girarda reviewed Dec 7, 2022
View reviewed changes
airbyte-integrations/connectors/source-monday/source_monday/dpath_string_extractor.py


@dataclass
class DpathStringExtractor(RecordExtractor, JsonSchemaMixin):
Copy link
Contributor

Choose a reason for hiding this comment

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

@roman-yermilov-gl can you add a comment explaining how this behaves differently from the default DPathExtractor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

airbyte-integrations/connectors/source-monday/source_monday/pagination_workaround.py Outdated


@dataclass
class PageIncrementWorkaround(PaginationStrategy, JsonSchemaMixin):
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need this workaround?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need page_size to be optional and to be calculated on fly for those cases when we don't know exactly how many records we have for some endpoint. We also need page to start from 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed custom component. Managed it with default pagination strategy

girarda
girarda reviewed Dec 7, 2022
View reviewed changes
airbyte-integrations/connectors/source-monday/source_monday/schemas/boards.json
@@ -27,7 +27,7 @@
"title": { "type": ["null", "string"] }
}
},
"id": { "type": ["null", "integer"] },
"id": { "type": ["null", "string"] },
Copy link
Contributor

Choose a reason for hiding this comment

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

changing the field types is a breaking change. we'll need to coordinate with TCS on releasing this if it's used in Cloud. cc @YowanR

@roman-yermilov-gl roman-yermilov-gl requested review from a team as code owners December 8, 2022 14:21
@roman-yermilov-gl roman-yermilov-gl added area/documentation Improvements or additions to documentation area/connectors Connector related issues labels Dec 8, 2022
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets December 8, 2022 16:55 — with GitHub Actions Inactive
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets December 8, 2022 16:55 — with GitHub Actions Inactive
@roman-yermilov-gl roman-yermilov-gl force-pushed the ryermilov/source-monday-migration branch from 8e64845 to 028d553 Compare December 8, 2022 18:41
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets December 8, 2022 18:43 — with GitHub Actions Inactive
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets December 8, 2022 18:43 — with GitHub Actions Inactive
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets December 9, 2022 12:48 — with GitHub Actions Inactive
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets December 9, 2022 12:48 — with GitHub Actions Inactive
@roman-yermilov-gl roman-yermilov-gl force-pushed the ryermilov/source-monday-migration branch from 129aaa7 to ce1cd12 Compare December 9, 2022 12:50
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets December 9, 2022 12:52 — with GitHub Actions Inactive
@roman-yermilov-gl roman-yermilov-gl temporarily deployed to more-secrets December 9, 2022 12:52 — with GitHub Actions Inactive
@roman-yermilov-gl
Copy link
Contributor Author

roman-yermilov-gl commented Dec 12, 2022 •
edited by github-actions bot
Loading

/test connector=connectors/source-monday

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

Name                                                Stmts   Miss  Cover
-----------------------------------------------------------------------
source_monday/__init__.py                               4      0   100%
source_monday/graphql_request_options_provider.py      58      5    91%
source_monday/dpath_string_extractor.py                22      4    82%
source_monday/source_lc.py                              4      1    75%
source_monday/source.py                               102     46    55%
-----------------------------------------------------------------------
TOTAL                                                 190     56    71%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       140      5    96%   87, 93, 238, 242-243
	 source_acceptance_test/conftest.py                     208     92    56%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-280, 288-301, 306-312, 319-330, 337-353
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              398    111    72%   53, 58, 87-95, 100-107, 111-112, 116-117, 299, 337-354, 363-371, 375-380, 386, 419-424, 462-469, 512-514, 517, 582-590, 602-605, 610, 666-667, 673, 676, 712-722, 735-760
	 source_acceptance_test/tests/test_incremental.py       158     14    91%   52-59, 64-77, 240
	 source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1599    332    79%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: Incremental syncs are not supported on this connector.
SKIPPED [3] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:370: Backward compatibility tests are disabled for version 0.1.4.
================== 47 passed, 4 skipped in 262.29s (0:04:22) ===================

@roman-yermilov-gl
Copy link
Contributor Author

roman-yermilov-gl commented Dec 13, 2022 •
edited by github-actions bot
Loading

/publish connector=connectors/source-monday

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


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

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

@roman-yermilov-gl roman-yermilov-gl merged commit f464b22 into master Dec 13, 2022
@roman-yermilov-gl roman-yermilov-gl deleted the ryermilov/source-monday-migration branch December 13, 2022 17:58
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets December 13, 2022 17:58 — with GitHub Actions Inactive
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets December 13, 2022 17:59 — with GitHub Actions Inactive
@github-actions github-actions bot mentioned this pull request Dec 15, 2022
Closed
@octavia-squidington-iii octavia-squidington-iii mentioned this pull request Dec 18, 2022
Merged
@tmvy tmvy mentioned this pull request Jan 2, 2023
Closed
@bazarnov bazarnov mentioned this pull request Jun 12, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@girarda girarda girarda approved these changes

@grubberr grubberr Awaiting requested review from grubberr

@arsenlosenko arsenlosenko Awaiting requested review from arsenlosenko

@artem1205 artem1205 Awaiting requested review from artem1205

@lazebnyi lazebnyi Awaiting requested review from lazebnyi

@darynaishchenko darynaishchenko Awaiting requested review from darynaishchenko

@davydov-d davydov-d Awaiting requested review from davydov-d

@brianjlai brianjlai Awaiting requested review from brianjlai

Assignees

@roman-yermilov-gl roman-yermilov-gl

Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/monday
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@roman-yermilov-gl @girarda @davydov-d @octavia-squidington-iii @octavia-squidington-iv