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

Include http method in SimpleRetriever log message for requests #19964

Merged
merged 3 commits into from
Dec 1, 2022

Conversation

clnoll
Copy link
Contributor

@clnoll clnoll commented Dec 1, 2022

What

Adds the HTTP method used by the request to logging output of the airbyte-cdk, per #19735.

How

Adds a "http_method": <request.method> key to AirbyteLogMessage's message dictionary.

Recommended reading order

  1. airbyte-cdk/python/airbyte_cdk/sources/declarative/retrievers/simple_retriever.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.

  • Unit & integration tests passing.
  • Code reviews completed
  • PR name follows PR naming conventions
  • Issue acceptance criteria met*

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

@clnoll clnoll requested a review from a team as a code owner December 1, 2022 04:26
@CLAassistant
Copy link

CLAassistant commented Dec 1, 2022

CLA assistant check
All committers have signed the CLA.

@octavia-squidington-iv octavia-squidington-iv added the CDK Connector Development Kit label Dec 1, 2022
@clnoll clnoll requested a review from girarda December 1, 2022 04:27
@clnoll clnoll force-pushed the airbyte-cdk-add-http-method-to-logs branch 2 times, most recently from 1de7aae to e10ccdc Compare December 1, 2022 15:48
Copy link
Contributor

@girarda girarda left a comment

Choose a reason for hiding this comment

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

lgtm!

next steps will be:

  1. Bump the version
  2. Update the changelog
  3. /publish-cdk dry-run=true
  4. /publish-cdk dry-run=false
  5. Merge 🎉

@clnoll clnoll force-pushed the airbyte-cdk-add-http-method-to-logs branch from e10ccdc to daad12e Compare December 1, 2022 19:34
@clnoll
Copy link
Contributor Author

clnoll commented Dec 1, 2022

/publish-cdk dry-run=true

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

@clnoll
Copy link
Contributor Author

clnoll commented Dec 1, 2022

/publish-cdk dry-run=false

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

@clnoll clnoll merged commit 44a1440 into master Dec 1, 2022
@clnoll clnoll deleted the airbyte-cdk-add-http-method-to-logs branch December 1, 2022 20:52
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.

4 participants