fix: update log in DpathValidator
#625
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the log messages in DpathValidator for improved clarity and adjusts the log level from warning to info.
- Change log level in except blocks from warning to info
- Update log message text for both list and singular key error validations
Comments suppressed due to low confidence (2)
airbyte_cdk/sources/declarative/validators/dpath_validator.py:56
- Switching the log level from warning to info reduces alert visibility. Please confirm that this change aligns with our error monitoring strategy to ensure key errors are adequately tracked in production.
logger.info(f"Validation skipped. Key not found: {e}")
airbyte_cdk/sources/declarative/validators/dpath_validator.py:64
- Switching the log level from warning to info reduces alert visibility. Please confirm that this change aligns with our error monitoring strategy to ensure key errors are adequately tracked in production.
logger.info(f"Validation skipped. Key not found: {e}")
📝 WalkthroughWalkthroughThe logging level for Changes
Possibly related PRs
Suggested labels
Would you like to consider these labels, or is there another you think fits better? Wdyt? ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
airbyte_cdk/sources/declarative/validators/dpath_validator.py (1)
55-57: Consider downgrading todebugand deduplicating the log statement, wdyt?
KeyErroris an entirely expected control-flow outcome when optional fields are absent; emitting it atinfomay add noise for users polling INFO logs. Adebuglevel (or even silencing it) would still allow troubleshooting while keeping default logs clean.
Additionally, the identical message appears in both wildcard and non-wildcard branches—extracting it into a small helper (or logging once before early-returning) could DRY things up.- logger.info(f"Validation skipped. Key not found: {e}") + logger.debug("Validation skipped. Key not found: %s", e)This change would also leverage the logger’s lazy string interpolation.
Happy to open a follow-up patch if you agree!Also applies to: 63-65
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airbyte_cdk/sources/declarative/validators/dpath_validator.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ChristoGrab
PR: airbytehq/airbyte-python-cdk#58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the `YamlDeclarativeSource` class in `airbyte_cdk/sources/declarative/yaml_declarative_source.py`, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
🧬 Code Graph Analysis (1)
airbyte_cdk/sources/declarative/validators/dpath_validator.py (3)
unit_tests/sources/declarative/validators/test_dpath_validator.py (1)
validate(18-22)airbyte_cdk/sources/declarative/validators/validation_strategy.py (1)
validate(15-22)airbyte_cdk/sources/declarative/validators/validator.py (1)
validate(11-18)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: source-amplitude
- GitHub Check: Check: source-intercom
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-shopify
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: SDM Docker Image Build
- GitHub Check: Pytest (Fast)
What
Summary by CodeRabbit
Important
Auto-merge enabled.
This PR is set to merge automatically when all requirements are met.