SimpleRetriever yield request and response as log messages #18644

girarda · 2022-10-29T00:21:51Z

What

This is a follow up PR to Stream returns AirbyteMessages #18572
Update SimpleRetriever.read_records so it also yield the raw requests and responses as AirbyteLogMessages if running in debug mode

How

Extract a parameterized _read_pages method from the HttpStream. The method reads the pages of records and is parameterized by a record generator function
The default record generation function is HttpStream.parse_response
The SimpleRetriever uses a custom record generator which also yields AirbyteLogMessages containing the raw request and response
Update the declarative stream so it only applies transforms on record, and not on log or trace messages
Add a debug field to the ManifestDeclarativeSource which can be used to set the log level to debug

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

Community member? 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
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
- docs/integrations/README.md
- airbyte-integrations/builds.md
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 by running the /publish command described here
After the connector is published, connector added to connector index as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

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
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. 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

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

…18689)" This reverts commit bf06dec.

girarda · 2022-11-09T06:15:57Z

/test connector=connectors/source-courier

🕑 connectors/source-courier https://github.com/airbytehq/airbyte/actions/runs/3425793661
❌ connectors/source-courier https://github.com/airbytehq/airbyte/actions/runs/3425793661
🐛 https://gradle.com/s/kvhdpn4f65nby

Build Failed

Test summary info:

Could not find result summary

girarda · 2022-11-09T05:52:39Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/manifest_declarative_source.py

 return super().spec(logger)

+ def check(self, logger: logging.Logger, config: Mapping[str, Any]) -> AirbyteConnectionStatus:
+ self._configure_logger_level(logger)


set log level to debug if debug mode is enabled from the constructor

girarda · 2022-11-09T06:32:12Z

/test connector=connectors/source-courier

🕑 connectors/source-courier https://github.com/airbytehq/airbyte/actions/runs/3425864829
❌ connectors/source-courier https://github.com/airbytehq/airbyte/actions/runs/3425864829
🐛 https://gradle.com/s/xdjnlachjumfu

Build Failed

Test summary info:

Could not find result summary

girarda · 2022-11-09T06:41:14Z

/test connector=connectors/source-courier

🕑 connectors/source-courier https://github.com/airbytehq/airbyte/actions/runs/3425904574
❌ connectors/source-courier https://github.com/airbytehq/airbyte/actions/runs/3425904574
🐛 https://gradle.com/s/pu6vzphg24b2o

Build Failed

Test summary info:

Could not find result summary

girarda · 2022-11-09T06:43:03Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/stream_slicers/substream_slicer.py

 sync_mode=SyncMode.full_refresh, cursor_field=None, stream_slice=parent_stream_slice, stream_state=None
 ):
+ # Skip non-records (eg AirbyteLogMessage)
+ if isinstance(parent_record, AirbyteMessage):


need to check the type of the output of read_records now

girarda · 2022-11-09T06:56:01Z

/test connector=connectors/source-courier

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

 Name Stmts Miss Cover Missing ---------------------------------------------------------------------------------- source_acceptance_test/base.py 12 4 67% 16-19 source_acceptance_test/config.py 133 3 98% 87, 93, 230 source_acceptance_test/conftest.py 196 92 53% 35, 41-43, 48, 54, 60, 66, 72-74, 93, 98-100, 106-108, 114-115, 120-121, 126, 132, 141-150, 156-161, 176, 200, 231, 237, 243-248, 256-261, 269-282, 287-293, 300-311, 318-334 source_acceptance_test/plugin.py 69 25 64% 22-23, 31, 36, 120-140, 144-148 source_acceptance_test/tests/test_core.py 345 110 68% 53, 64-72, 77-84, 88-89, 93-94, 178, 216-233, 242-250, 254-259, 265, 298-303, 341-348, 391-393, 396, 461-469, 481-484, 489, 545-546, 552, 555, 591-601, 614-639 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 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/config_migration.py 23 23 0% 5-37 source_acceptance_test/utils/connector_runner.py 112 50 55% 23-26, 32, 36, 39-68, 71-73, 76-78, 81-83, 86-88, 91-93, 96-114, 148-150 source_acceptance_test/utils/json_schema_helper.py 105 13 88% 30-31, 38, 41, 65-68, 96, 120, 190-192 ---------------------------------------------------------------------------------- TOTAL 1512 375 75%

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: not found in the config. ================= 26 passed, 1 skipped, 27 warnings in 35.98s ==================

This reverts commit 85979a8.

This reverts commit a8a8a2d.

girarda · 2022-11-09T07:10:59Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/manifest_declarative_source.py

 VALID_TOP_LEVEL_FIELDS = {"check", "definitions", "spec", "streams", "version"}

- def __init__(self, source_config: ConnectionDefinition):
+ def __init__(self, source_config: ConnectionDefinition, debug: bool = False):


@brianjlai the connector-builder server can set this field to true

sherifnada · 2022-11-09T17:15:28Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/manifest_declarative_source.py

+ catalog: ConfiguredAirbyteCatalog,
+ state: Union[List[AirbyteStateMessage], MutableMapping[str, Any]] = None,
+ ) -> Iterator[AirbyteMessage]:
+ self._configure_logger_level(logger)


for the next breaking change, i feel like we should remove the logger from these parameters. It should be bound to the instance.

sherifnada · 2022-11-09T17:32:42Z

airbyte-cdk/python/airbyte_cdk/sources/declarative/manifest_declarative_source.py

+ catalog: ConfiguredAirbyteCatalog,
+ state: Union[List[AirbyteStateMessage], MutableMapping[str, Any]] = None,
+ ) -> Iterator[AirbyteMessage]:
+ self._configure_logger_level(logger)


Created a CDK v2 wishlist #19239

brianjlai

looks good and the log messages should integrate well with the code i have for the connector builder server.

* method yielding airbytemessage * move to Stream * update abstract source * reset * missing file * Yield request and response as log messages * only emit request and responses if the debug flag is on * add test docker image * script to run acceptance tests with local cdk * Update conftest to use a different image * extract to method * dont use a different image tag * Always install local cdk * break the cdk * get path from current working directory * or * ignore unit test * debug log * Revert "AMI change: ami-0f23be2f917510c26 -> ami-005924fb76f7477ce (#18689)" This reverts commit bf06dec. * build from the top * Update source-acceptance-test * fix * copy setup * some work on the gradle plugin * reset to master * delete unused file * delete unused file * reset to master * optional argument * delete dead code * use latest cdk with sendgrid * fix sendgrid dockerfile * break the cdk * use local file * Revert "break the cdk" This reverts commit 600c195. * dont raise an exception * reset to master * unit tests * missing test * more unit tests * remove deprecated comment * newline * reset to master * remove files * reset * Update abstract source * remove method from stream * convert to airbytemessage * unittests * Update * unit test * remove debug logs * Revert "remove debug logs" This reverts commit a1a139e. * Revert "Revert "remove debug logs"" This reverts commit b1d62cd. * Revert "reset to master" This reverts commit 3fa6a00. * fix * slightly better test * typing * extract method * Revert "Revert "reset to master"" This reverts commit 5dac7c2. * reset to master * reset to master * Revert "reset to master" This reverts commit 3fa6a00. * Comment * operate on the message * Revert "Revert "reset to master"" This reverts commit 5833c84. * comment * test * Revert "test" This reverts commit 2f91b80. * test * Revert "test" This reverts commit 62d95eb. * test * Revert "test" This reverts commit 27150ba. * format * format * symlink * Update setup * update path * reset to master * update * Add local files * greenhouse * format * symlink * try reordering * better error message * better log message * reset to master * Revert "merge for qa" This reverts commit ad7128f, reversing changes made to 7196c22. * reset to master * reset to master * reset to master * format * gradlew format * right type hints * reset to master * reset to master * gradlew format * a bunch of small fixes * Update output format * fixes from feedback * fixme comment * streams cannot return AirbyteRecordMessage * fix * format * only return logs when running on debug mode * move branching * update typing * remove dead code * fix simpleretriever name * i think this is better * log response.text * debug flag * comment * pass config * comments * run SATs * fix most of the unit tests * fix unit test * reset to master * runFromPath * Revert "runFromPath" This reverts commit 85979a8. * Revert "run SATs" This reverts commit a8a8a2d. * no need to convert to dict * fix test

girarda added 6 commits October 27, 2022 12:58

method yielding airbytemessage

5f88aa6

move to Stream

0de8395

update abstract source

e5acdcf

reset

8a63af7

missing file

d0dcb52

Yield request and response as log messages

d021da6

github-actions bot added the CDK Connector Development Kit label Oct 29, 2022

girarda added 23 commits October 28, 2022 17:35

only emit request and responses if the debug flag is on

dc9ff7b

add test docker image

adf07fa

script to run acceptance tests with local cdk

c68e01d

Update conftest to use a different image

258990d

extract to method

b0f80a7

dont use a different image tag

68528b2

Always install local cdk

b31f9bd

break the cdk

893beab

get path from current working directory

c3964c9

or

c69774a

ignore unit test

152f5df

debug log

8ea5e85

Revert "AMI change: ami-0f23be2f917510c26 -> ami-005924fb76f7477ce (#…

1684823

…18689)" This reverts commit bf06dec.

build from the top

b08a9c4

Update source-acceptance-test

4fd9ed0

fix

06b1c70

copy setup

3e4b135

some work on the gradle plugin

4702f4d

reset to master

ae7b7fb

delete unused file

3d86bce

delete unused file

9dc4486

reset to master

d3852f3

optional argument

179d689

run SATs

a8a8a2d

girarda temporarily deployed to more-secrets November 9, 2022 06:16 Inactive

girarda added 2 commits November 8, 2022 22:25

fix most of the unit tests

9060419

fix unit test

64bb445

girarda commented Nov 9, 2022

View reviewed changes

girarda temporarily deployed to more-secrets November 9, 2022 06:33 Inactive

reset to master

e9a867c

girarda commented Nov 9, 2022

View reviewed changes

girarda temporarily deployed to more-secrets November 9, 2022 06:43 Inactive

runFromPath

85979a8

girarda temporarily deployed to more-secrets November 9, 2022 06:58 Inactive

girarda added 2 commits November 8, 2022 23:09

Revert "runFromPath"

846bed7

This reverts commit 85979a8.

Revert "run SATs"

b114add

This reverts commit a8a8a2d.

girarda commented Nov 9, 2022

View reviewed changes

no need to convert to dict

adfb495

girarda requested review from brianjlai and sherifnada November 9, 2022 07:14

fix test

3dbeeb3

sherifnada approved these changes Nov 9, 2022

View reviewed changes

brianjlai approved these changes Nov 9, 2022

View reviewed changes

girarda merged commit e029353 into master Nov 9, 2022

girarda deleted the alex/read_with_logs_lowcode branch November 9, 2022 21:26

brianjlai mentioned this pull request Nov 9, 2022

[low-code] publish the cdk #19255

Merged

github-actions bot mentioned this pull request Nov 15, 2022

Bump helm chart version reference to 0.40.43 #19453

Closed

octavia-squidington-iii mentioned this pull request Nov 18, 2022

Bump Airbyte version from 0.40.18 to 0.40.19 #19579

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

SimpleRetriever yield request and response as log messages #18644

SimpleRetriever yield request and response as log messages #18644

Uh oh!

girarda commented Oct 29, 2022 •

edited

Loading

girarda commented Nov 9, 2022 •

edited by github-actions bot

Loading

girarda Nov 9, 2022

girarda commented Nov 9, 2022 •

edited by github-actions bot

Loading

girarda commented Nov 9, 2022 •

edited by github-actions bot

Loading

girarda Nov 9, 2022

girarda commented Nov 9, 2022 •

edited by github-actions bot

Loading

girarda Nov 9, 2022

sherifnada Nov 9, 2022

sherifnada Nov 9, 2022

brianjlai left a comment

Labels

5 participants

SimpleRetriever yield request and response as log messages #18644

SimpleRetriever yield request and response as log messages #18644

Uh oh!

Conversation

girarda commented Oct 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

girarda commented Nov 9, 2022 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Failed

girarda Nov 9, 2022

Choose a reason for hiding this comment

girarda commented Nov 9, 2022 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Failed

girarda commented Nov 9, 2022 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Failed

girarda Nov 9, 2022

Choose a reason for hiding this comment

girarda commented Nov 9, 2022 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Passed

girarda Nov 9, 2022

Choose a reason for hiding this comment

sherifnada Nov 9, 2022

Choose a reason for hiding this comment

sherifnada Nov 9, 2022

Choose a reason for hiding this comment

brianjlai left a comment

Choose a reason for hiding this comment

Labels

5 participants

girarda commented Oct 29, 2022 •

edited

Loading

girarda commented Nov 9, 2022 •

edited by github-actions bot

Loading

girarda commented Nov 9, 2022 •

edited by github-actions bot

Loading

girarda commented Nov 9, 2022 •

edited by github-actions bot

Loading

girarda commented Nov 9, 2022 •

edited by github-actions bot

Loading