-
Notifications
You must be signed in to change notification settings - Fork 50
Improve logging for load_file #1312
Conversation
tatiana
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@feluelle would it make sense to have a test covering this change? Approving it regardless, since the change already helps users.
Codecov ReportBase: 94.91% // Head: 95.71% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1312 +/- ##
==========================================
+ Coverage 94.91% 95.71% +0.79%
==========================================
Files 52 19 -33
Lines 2617 677 -1940
Branches 311 68 -243
==========================================
- Hits 2484 648 -1836
+ Misses 82 18 -64
+ Partials 51 11 -40 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
kaxil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One test failed for Airflow 2.4 and 2.2.5:
google.api_core.exceptions.BadRequest: 400 Provided Schema does not match Table astronomer-dag-authoring:first_table_schema._tmp_gcuqy9jbp3wxal8q0uqwb8jrk2a2zr6zage4t0g. Field id has changed mode from REQUIRED to NULLABLE
da7b0d3 to
8574062
Compare
#1324) # Description ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> - Add the `NATIVE_LOAD_EXCEPTIONS` to BaseDatabase as it is being overloaded. <!-- Issues are required for both bug fixes and features. Reference it using one of the following: closes: #ISSUE related: #ISSUE --> related to #1312 as change was not correct ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Add the `NATIVE_LOAD_EXCEPTIONS` to BaseDatabase as it is being overloaded. ## Does this introduce a breaking change? No ### Checklist - [x] Created tests which fail without the change (if possible) - [x] Extended the README / documentation, if necessary
# Description ## What is the current behavior? Currently, we don't have any logging for native nor pandas _load_file_. This makes it harder for users to follow the operations. related: #1263 ## What is the new behavior? We now have log statements for native, pandas as well as fallback indication. ## Does this introduce a breaking change? No. ### Checklist - [ ] Created tests which fail without the change (if possible) - [ ] Extended the README / documentation, if necessary (cherry picked from commit 279c7d1)
#1324) # Description ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> - Add the `NATIVE_LOAD_EXCEPTIONS` to BaseDatabase as it is being overloaded. <!-- Issues are required for both bug fixes and features. Reference it using one of the following: closes: #ISSUE related: #ISSUE --> related to #1312 as change was not correct ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Add the `NATIVE_LOAD_EXCEPTIONS` to BaseDatabase as it is being overloaded. ## Does this introduce a breaking change? No ### Checklist - [x] Created tests which fail without the change (if possible) - [x] Extended the README / documentation, if necessary (cherry picked from commit 0c64f48)




Description
What is the current behavior?
Currently, we don't have any logging for native nor pandas load_file. This makes it harder for users to follow the operations.
related: #1263
What is the new behavior?
We now have log statements for native, pandas as well as fallback indication.
Does this introduce a breaking change?
No.
Checklist