fix(warehouses): Add write_dispositions to each resource#180
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe pull request refactors write disposition handling across multiple extract-load modules, transitioning from global default parameters to targeted resource-level specifications via apply_hints and resource_defaults configurations. Documentation updates accompany these structural changes. Changes
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
Fix problems arising from removal of cli parameter
961ba6f to
4fba11b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In
@warehouses/accelerator/extract_load/accelerator_sharepoint/extract_and_load.py:
- Around line 58-59: The write_disposition entries are using dicts instead of
simple strings; change the two occurrences where
write_disposition={"disposition": "replace"} to write_disposition="replace"
(e.g., in the load calls for edr_equipment and edr_equipment_mapping inside
extract_and_load.py) so they match the simple-string dlt syntax used elsewhere
and reserve dict form only for dispositions with extra params like
merge/strategy.
🧹 Nitpick comments (2)
warehouses/accelerator/extract_load/accelerator_sharepoint/extract_and_load.py (1)
73-81: Simplify write_disposition to string format in apply_hints.The
write_dispositionparameter uses a dictionary format{"disposition": "replace"}, but since there is no additional configuration (such as merge strategy), the simpler string format"replace"is clearer and more consistent with other simple write disposition usages in the codebase.Suggested fix
.with_name("edr_equipment_mapping") .apply_hints( - write_disposition={"disposition": "replace"}, + write_disposition="replace", )warehouses/accelerator/extract_load/electricity_sharepoint/extract_and_load.py (1)
158-160: Simplify write_disposition to use string format.The write_disposition parameter accepts both formats: a simple string (
"merge") or a structured dict ({"disposition": "merge", "strategy": "upsert"}). The dict format is intended for specifying additional merge strategies; since no strategy is specified here, the simpler string format is more appropriate.- yield from reader.apply_hints( - write_disposition={"disposition": "merge"}, - ) + yield from reader.apply_hints( + write_disposition="merge", + )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (4)
elt-common/src/elt_common/cli.pywarehouses/accelerator/extract_load/accelerator_sharepoint/extract_and_load.pywarehouses/accelerator/extract_load/electricity_sharepoint/extract_and_load.pywarehouses/accelerator/extract_load/statusdisplay/extract_and_load.py
💤 Files with no reviewable changes (1)
- elt-common/src/elt_common/cli.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: elt-common end-to-end tests
- GitHub Check: warehouses end-to-end tests
🔇 Additional comments (1)
warehouses/accelerator/extract_load/statusdisplay/extract_and_load.py (1)
28-30: Correct approach for setting resource defaults.The
resource_defaultsconfiguration properly sets the write disposition for all resources from this REST API source. The string format "replace" is correct.
1 participant
Summary
Fix problems arising from removal of cli parameter in #177. Set write_dispositions explicitly on each resource.
Summary by CodeRabbit
Documentation
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.