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

🐛 Airbyte CDK: transforming Python log levels to Airbyte protocol log levels #7024

Merged
merged 5 commits into from
Oct 22, 2021

Conversation

vitaliizazmic
Copy link
Contributor

What

Improving PR #6085. Remove adding Python log levels to Airbyte protocol, instead transform them to the Airbyte protocol log levels.

How

Changed method "format" of AirbyteLogFormatter. Mapping Python log levels with Airbyte protocol log levels.

@vitaliizazmic vitaliizazmic added the CDK Connector Development Kit label Oct 13, 2021
@vitaliizazmic vitaliizazmic self-assigned this Oct 13, 2021
Copy link
Contributor

@davinchia davinchia left a comment

Choose a reason for hiding this comment

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

Thank you for the fast turnaround!

I remember you added tests for log levels in the previous PR. Is this change reflected in those tests?

@vitaliizazmic
Copy link
Contributor Author

@davinchia I will create additional test.

@vitaliizazmic vitaliizazmic temporarily deployed to more-secrets October 18, 2021 12:20 Inactive
@@ -44,10 +44,8 @@ class Config:

class Level(Enum):
FATAL = "FATAL"
CRITICAL = "CRITICAL"
Copy link
Contributor

Choose a reason for hiding this comment

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

what about the critical log level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davinchia I transform levels by them numeric value. CRITICAL numeric value is equal to FATAL numeric value and will be transform to FATAL.

Copy link
Contributor

Choose a reason for hiding this comment

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

do we have a test for that I am missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davinchia I've added test for transforming CRITICAL level

@vitaliizazmic vitaliizazmic temporarily deployed to more-secrets October 20, 2021 12:07 Inactive
# Conflicts:
#	airbyte-cdk/python/CHANGELOG.md
#	airbyte-cdk/python/setup.py
@vitaliizazmic vitaliizazmic temporarily deployed to more-secrets October 22, 2021 13:31 Inactive
@vitaliizazmic
Copy link
Contributor Author

vitaliizazmic commented Oct 22, 2021

/publish-cdk dry-run=false

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

@vitaliizazmic vitaliizazmic merged commit 0c7f060 into master Oct 22, 2021
@vitaliizazmic vitaliizazmic deleted the vitalii/1279_cdk_standard_logger_levels branch October 22, 2021 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/protocol CDK Connector Development Kit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants