Lazy load neo4j driver imports#67522
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
|
dwreeves
left a comment
There was a problem hiding this comment.
Seems good to me. Thanks!
|
Hi @Anuragp22, thank you for the focused work here -- the engineering on this PR is clean. I'm closing this along with the other PRs targeting #67515 after thinking through the actual savings more carefully. Full reasoning here: #67515 (comment) Short version: an Operator imports a Hook which imports the SDK, so moving the SDK import inside a method mostly shifts when the import happens rather than saving it -- workers running the task pay the same cost. It also loses the parse-time Genuinely sorry for the wasted cycles -- on us for not flagging the strategy concern before contributors started shipping. Two areas that are worth the work if you want something else to pick up:
Thanks again. |
Lazy-load
neo4jSDK imports to reduce memory footprint and import time ofairflow.providers.neo4j.hooks.neo4j, following the same pattern as #62365 (snowflake) and #67519 (elasticsearch).Hook changes
from neo4j import Driver, GraphDatabase.Driveris only used as a type annotation, so it now lives underif TYPE_CHECKING:.GraphDatabase.driver(...)is only invoked inside_create_driver, so the import is moved into that method body.Test changes
mock.patchtargets intest_neo4j.pyfromairflow.providers.neo4j.hooks.neo4j.GraphDatabasetoneo4j.GraphDatabase(and...GraphDatabase.driverfor the parametrized test), sinceGraphDatabaseis no longer a module-level symbol in the hook module.Validation
Local smoke check (Windows host; full unit suite requires the Linux-only
fcntlimport that Airflow's test conftest pulls in transitively):Neo4jHookno longer loads theneo4jpackage intosys.modules.DriverandGraphDatabaseare no longer attributes of the hook module.neo4j.GraphDatabaseandneo4j.GraphDatabase.driverresolve asmock.patchtargets.The unit tests in
providers/neo4j/tests/unit/neo4j/hooks/test_neo4j.pyshould now pass against the moved import; CI will be the source of truth here.Was generative AI tooling used to co-author this PR?