SimpleRetriever yield request and response as log messages #18644
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/test connector=connectors/source-courier
Build FailedTest summary info: |
girarda
commented
Nov 9, 2022
| @@ -84,6 +93,27 @@ def spec(self, logger: logging.Logger) -> ConnectorSpecification: | |||
| else: | |||
| return super().spec(logger) | |||
|
|
|||
| def check(self, logger: logging.Logger, config: Mapping[str, Any]) -> AirbyteConnectionStatus: | |||
| self._configure_logger_level(logger) | |||
There was a problem hiding this comment.
set log level to debug if debug mode is enabled from the constructor
|
/test connector=connectors/source-courier
Build FailedTest summary info: |
|
/test connector=connectors/source-courier
Build FailedTest summary info: |
girarda
commented
Nov 9, 2022
| @@ -138,6 +138,12 @@ def stream_slices(self, sync_mode: SyncMode, stream_state: StreamState) -> Itera | |||
| for parent_record in parent_stream.read_records( | |||
| 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): | |||
There was a problem hiding this comment.
need to check the type of the output of read_records now
|
/test connector=connectors/source-courier
Build PassedTest summary info: |
girarda
commented
Nov 9, 2022
| @@ -35,12 +41,14 @@ class ManifestDeclarativeSource(DeclarativeSource): | |||
|
|
|||
| VALID_TOP_LEVEL_FIELDS = {"check", "definitions", "spec", "streams", "version"} | |||
|
|
|||
| def __init__(self, source_config: ConnectionDefinition): | |||
| def __init__(self, source_config: ConnectionDefinition, debug: bool = False): | |||
There was a problem hiding this comment.
@brianjlai the connector-builder server can set this field to true
sherifnada
approved these changes
Nov 9, 2022
| catalog: ConfiguredAirbyteCatalog, | ||
| state: Union[List[AirbyteStateMessage], MutableMapping[str, Any]] = None, | ||
| ) -> Iterator[AirbyteMessage]: | ||
| self._configure_logger_level(logger) |
There was a problem hiding this comment.
for the next breaking change, i feel like we should remove the logger from these parameters. It should be bound to the instance.
| catalog: ConfiguredAirbyteCatalog, | ||
| state: Union[List[AirbyteStateMessage], MutableMapping[str, Any]] = None, | ||
| ) -> Iterator[AirbyteMessage]: | ||
| self._configure_logger_level(logger) |
There was a problem hiding this comment.
Created a CDK v2 wishlist #19239
brianjlai
approved these changes
Nov 9, 2022
akashkulk
pushed a commit
that referenced
this pull request
Dec 2, 2022
* 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
How
_read_pagesmethod from the HttpStream. The method reads the pages of records and is parameterized by a record generator functionSimpleRetrieveruses a custom record generator which also yields AirbyteLogMessages containing the raw request and responseManifestDeclarativeSourcewhich can be used to set the log level to debugRecommended reading order
airbyte-cdk/python/airbyte_cdk/sources/streams/http/http.pyairbyte-cdk/python/airbyte_cdk/sources/declarative/retrievers/simple_retriever.pyairbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/request.pyairbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/response.pyairbyte-cdk/python/airbyte_cdk/sources/declarative/retrievers/retriever.pyairbyte-cdk/python/airbyte_cdk/sources/declarative/declarative_stream.pyairbyte-cdk/python/airbyte_cdk/sources/utils/record_helper.py🚨 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
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampledocs/integrations/README.mdairbyte-integrations/builds.mdAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing/publishcommand described hereUpdating a connector
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing/publishcommand described hereConnector Generator
-scaffoldin their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplatesthen checking in your changesTests
Unit
Put your unit tests output here.
Integration
Put your integration tests output here.
Acceptance
Put your acceptance tests output here.