-
Notifications
You must be signed in to change notification settings - Fork 14.6k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix(hive): Fix regression from #21943 #22431
Conversation
table=table, | ||
), | ||
) | ||
with cls.get_engine(database) as engine: |
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.
This line is new. The rest is merely indented.
Codecov Report
@@ Coverage Diff @@
## master #22431 +/- ##
===========================================
- Coverage 66.90% 55.54% -11.36%
===========================================
Files 1850 1850
Lines 70692 70693 +1
Branches 7752 7752
===========================================
- Hits 47294 39269 -8025
- Misses 21382 29408 +8026
Partials 2016 2016
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@john-bodley I've run into several problems with mypy lately where it's not picking up trivial typing errors. I wasn't able to look into it more closely, but I did try bumping mypy locally and even enabling all checks, but it still kept glossing over the errors. |
SUMMARY
This PR fixes a regression introduced in #21943 where no context manger was used to create the engine, thus preventing users from being able to upload a CSV file via Hive.
I thought both Pylint would have detected a "Local variable referenced before assignment" error, but maybe the if/context manager logic was simply too complex, as well as the unit tests—but alas this doesn't seem to be the case.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
Tested locally.
ADDITIONAL INFORMATION