-
Notifications
You must be signed in to change notification settings - Fork 14.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add basic asyncio support #43944
base: main
Are you sure you want to change the base?
Add basic asyncio support #43944
Changes from all commits
84f6fb9
fcb930d
5dfbaf0
3de48eb
97546da
a99a393
de31c5d
00fbf35
24bbf30
60cce2c
6791e41
ca7b9b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -31,6 +31,7 @@ | |||
import pluggy | ||||
from packaging.version import Version | ||||
from sqlalchemy import create_engine, exc, text | ||||
from sqlalchemy.ext.asyncio import AsyncEngine, AsyncSession, create_async_engine | ||||
from sqlalchemy.orm import scoped_session, sessionmaker | ||||
from sqlalchemy.pool import NullPool | ||||
|
||||
|
@@ -95,8 +96,17 @@ | |||
DONOT_MODIFY_HANDLERS: bool | None = None | ||||
DAGS_FOLDER: str = os.path.expanduser(conf.get_mandatory_value("core", "DAGS_FOLDER")) | ||||
|
||||
AIO_LIBS_MAPPING = {"sqlite": "aiosqlite", "postgresql": "asyncpg", "mysql": "aiomysql"} | ||||
""" | ||||
Mapping of sync scheme to async scheme. | ||||
:meta private: | ||||
""" | ||||
|
||||
engine: Engine | ||||
Session: Callable[..., SASession] | ||||
async_engine: AsyncEngine | ||||
create_async_session: Callable[..., AsyncSession] | ||||
|
||||
# The JSON library to use for DAG Serialization and De-Serialization | ||||
json = json | ||||
|
@@ -199,13 +209,25 @@ def load_policy_plugins(pm: pluggy.PluginManager): | |||
pm.load_setuptools_entrypoints("airflow.policy") | ||||
|
||||
|
||||
def _get_async_conn_uri_from_sync(sync_uri): | ||||
scheme, rest = sync_uri.split(":", maxsplit=1) | ||||
scheme = scheme.split("+", maxsplit=1)[0] | ||||
aiolib = AIO_LIBS_MAPPING.get(scheme) | ||||
if aiolib: | ||||
return f"{scheme}+{aiolib}:{rest}" | ||||
else: | ||||
return sync_uri | ||||
|
||||
|
||||
def configure_vars(): | ||||
"""Configure Global Variables from airflow.cfg.""" | ||||
global SQL_ALCHEMY_CONN | ||||
global SQL_ALCHEMY_CONN_ASYNC | ||||
global DAGS_FOLDER | ||||
global PLUGINS_FOLDER | ||||
global DONOT_MODIFY_HANDLERS | ||||
SQL_ALCHEMY_CONN = conf.get("database", "SQL_ALCHEMY_CONN") | ||||
SQL_ALCHEMY_CONN_ASYNC = _get_async_conn_uri_from_sync(sync_uri=SQL_ALCHEMY_CONN) | ||||
|
||||
DAGS_FOLDER = os.path.expanduser(conf.get("core", "DAGS_FOLDER")) | ||||
|
||||
|
@@ -441,6 +463,9 @@ def configure_orm(disable_connection_pool=False, pool_class=None): | |||
|
||||
global Session | ||||
global engine | ||||
global async_engine | ||||
global create_async_session | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why @ashb ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think it's necessary ... it doesn't get set until configure_orm is called |
||||
|
||||
if os.environ.get("_AIRFLOW_SKIP_DB_TESTS") == "true": | ||||
# Skip DB initialization in unit tests, if DB tests are skipped | ||||
Session = SkipDBTestsSession | ||||
|
@@ -466,7 +491,14 @@ def configure_orm(disable_connection_pool=False, pool_class=None): | |||
connect_args["check_same_thread"] = False | ||||
|
||||
engine = create_engine(SQL_ALCHEMY_CONN, connect_args=connect_args, **engine_args, future=True) | ||||
|
||||
async_engine = create_async_engine(SQL_ALCHEMY_CONN_ASYNC, future=True) | ||||
create_async_session = sessionmaker( | ||||
bind=async_engine, | ||||
autocommit=False, | ||||
autoflush=False, | ||||
class_=AsyncSession, | ||||
expire_on_commit=False, | ||||
) | ||||
mask_secret(engine.url.password) | ||||
|
||||
setup_event_handlers(engine) | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -211,6 +211,7 @@ def test_get_documentation_package_path(): | |
""" | ||
"apache-airflow-providers-common-sql>=1.20.0b0", | ||
"apache-airflow>=2.8.0b0", | ||
"asyncpg>=0.30.0", | ||
"psycopg2-binary>=2.9.4", | ||
""", | ||
id="beta0 suffix postgres", | ||
|
@@ -221,6 +222,7 @@ def test_get_documentation_package_path(): | |
""" | ||
"apache-airflow-providers-common-sql>=1.20.0", | ||
"apache-airflow>=2.8.0", | ||
"asyncpg>=0.30.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. psycopg 3 also has async support but might conflict with psycopg 2 already installed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
True, yeah. We could in future look at allowing diff libs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Benchmark, so take with a pinch of salt, but https://pypi.org/project/asyncpg/
That's def a big enough of a change that we should see if we notice any difference |
||
"psycopg2-binary>=2.9.4", | ||
""", | ||
id="No suffix postgres", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
-e .[devel-devscripts,devel-tests,cncf.kubernetes] | ||
-e .[devel-devscripts,devel-tests,cncf.kubernetes,sqlite] | ||
-e ./providers | ||
-e ./task_sdk |
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.
SQLA has stuff built in to parse URLs/schemas. Maybe we should use that instead?
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.
https://docs.sqlalchemy.org/en/14/core/engines.html#sqlalchemy.engine.URL