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

example pokeapi with new protocol #17489

Closed
wants to merge 4 commits into from
Closed

example pokeapi with new protocol #17489

wants to merge 4 commits into from

Conversation

edgao
Copy link
Contributor

@edgao edgao commented Oct 1, 2022

An example port of source-pokeapi to the updated protocol version described in #17486. Specifically:

  • the schema is updated to reference well-known types
  • source.py uses the default type transformer to wrap numeric values into strings

This PR also demonstrates airbyte_attributes.

See comments below for further details - #17489 (review)

@github-actions github-actions bot added the area/connectors Connector related issues label Oct 1, 2022
@edgao edgao temporarily deployed to more-secrets October 1, 2022 00:19 Inactive
@edgao edgao temporarily deployed to more-secrets October 3, 2022 20:52 Inactive
},
"weight": {
"type": ["null", "integer"]
"$ref": "WellKnownTypes.json#definitions/Integer"
},
"abilities": {
"type": ["null", "array"],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

as seen here: non-primitive types (arrays, objects, oneof, etc) are not defined in WellKnownTypes, because they need additional configuration to fully describe their contents

@@ -0,0 +1,60 @@
{
Copy link
Contributor Author

@edgao edgao Oct 3, 2022

Choose a reason for hiding this comment

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

This file is just the JSON-ified version of WellKnownTypes.yaml. In the real usage, this file will be provided as part of the CDK - I'm copying it in manually because I don't want to set up the full yaml -> json -> copy pipeline just for this example.

@@ -3,45 +3,51 @@
"type": "object",
"properties": {
"id": {
"type": ["null", "integer"]
"$ref": "WellKnownTypes.json#definitions/Integer",
"airbyte_attributes": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here's an example of airbyte_attributes. The $ref technically "overrides" this, but that won't matter: All the components that care about airbyte_attributes don't actually want to resolve the $ref. For example, normalization would prefer to just check for $ref == [...]Integer rather than resolving it to the full {"type": "string", "pattern": "..."}.

This won't be available in our v0 implementation, but is something that we'll keep in mind for future enhancements. It's purely additive, so it'll be pretty easy as a bolt-on.

@@ -3,45 +3,51 @@
"type": "object",
"properties": {
"id": {
"type": ["null", "integer"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

null is redundant: these properties aren't required, so the source can just not set them at all.

If we find that sources actually want to output {"id": null}, then we can always update the well-known types to use type: [null, integer].

@edgao edgao marked this pull request as ready for review October 3, 2022 21:27
@evantahler
Copy link
Contributor

Does /test work? Give it a shot!

@edgao
Copy link
Contributor Author

edgao commented Oct 3, 2022

/test connector=connectors/source-pokeapi

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

	 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       152     26    83%   21-23, 29-31, 36-43, 48-61, 239, 250-258
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     17    78%   15-16, 24-30, 47-54, 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                                                 1358    466    66%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestSpec.test_config_match_spec because not found in the config
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestIncremental.test_two_sequential_reads because not found in the config
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:235: Backward compatibility tests are disabled for version 0.1.5.
======================== 12 passed, 3 skipped in 8.95s =========================

@edgao edgao temporarily deployed to more-secrets October 3, 2022 23:47 Inactive
Base automatically changed from edgao/data_type_protocol_proposal to master October 27, 2022 21:04
@edgao edgao closed this Nov 13, 2023
@edgao edgao deleted the edgao/data_types_demo branch November 13, 2023 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants