Add log lookup exception for empty op subtypes #35536
Conversation
|
@vchiapaikeo are you still working on this PR? |
vchiapaikeo
force-pushed
the
vchiapaikeo/empty-op-logs-v1
branch
from
e6b5756 to
5cb4fb0
Compare
|
Whoops, sorry about that. Rebased and ready for review here. I retested and it still works as expected - 
|
| import httpx | ||
|
|
There was a problem hiding this comment.
Why was this import moved to top of the file? is it due to pre commit change?
There was a problem hiding this comment.
No, I did this because I need this module on line 506 (httpx.UnsupportedProtocol). I looked at the blame and there didn't seem to be any reason for this to be function scoped so moving it to module scoped.
There was a problem hiding this comment.
Actually, it looks like Ash moves this out of module scope to function for perf optimization here: 1a8a897#diff-e7f34f73940eb52d92bb991abedc1c963431c5373c12dff739c8fb7d03e93d3aL24
Going to move it back to function scope in a follow up PR
* Add log lookup exception for empty op subtypes * Use exception catching approach instead to preserve tests (cherry picked from commit ddcaef4)
|
I believe this bug fix has been invalidated by #39799 it should be from requests.exceptions import InvalidURL |
original fix in apache#35536 invalidated by apache#39799
3 participants
closes: #34228
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.