Conversation
✅ Deploy Preview for sunny-pastelito-5ecb04 canceled.
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes mypy type checking issues that were causing CI failures on the main branch. The errors were related to incompatible type assignments involving ObjectStoragePath from Airflow's I/O module.
- Updated type annotations to use forward references for
ObjectStoragePathto handle optional dependencies - Added conditional imports using
TYPE_CHECKINGpattern to avoid runtime import errors - Fixed specific type annotation issues in cache, config, and operator modules
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| cosmos/operators/local.py | Added conditional import and updated return/parameter types for remote path functions |
| cosmos/dbt/graph.py | Added conditional import and updated parameter type for remote cache function |
| cosmos/config.py | Added conditional import and updated manifest_path and mandatory_paths type annotations |
| cosmos/cache.py | Added conditional import and updated return/variable types for remote cache directory configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
ObjectStoragePath in main branch
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
corsettigyg
left a comment
There was a problem hiding this comment.
apart from the import issue with airflow 3.x, everything looks good to me 💯
a possible (and clean fix) would be
from packaging.version import Version
AIRFLOW_VERSION = Version(airflow.__version__)
if AIRFLOW_VERSION >= Version("3.0"):
try:
from airflow.sdk import ObjectStoragePath
except ImportError:
pass
else:
try:
from airflow.io.path import ObjectStoragePath
except ImportError:
pass
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2012 +/- ##
=======================================
Coverage 97.80% 97.81%
=======================================
Files 87 87
Lines 5525 5526 +1
=======================================
+ Hits 5404 5405 +1
Misses 121 121 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
**Features** * Introduce ``ExecutionMode.WATCHER`` to reduce DAG run time by 1/5 in several PRs. Learn more about it [here](https://astronomer.github.io/astronomer-cosmos/getting_started/watcher-execution-mode.html#watcher-execution-mode). This feature was implemented via multiple PRs, including: * Expose new execution mode by @tatiana @pankajastro @pankajkoti in #1999 * Add ``DbtProducerWatcherOperator`` for the proposed ``ExecutionMode.WATCHER`` by @pankajkoti in #1982 * Add ``DbtConsumerWatcherSensor`` for the proposed ``ExecutionMode.WATCHER`` by @pankajastro in #1998 * Push producer's task completion status to XCOM by @pankajkoti in #2000 * Add default priority_weight for ``DbtProducerWatcherOperator`` by @pankajkoti in #1995 * Add sample dbt events for the dbt watcher execution mode by @pankajkoti in #1952 * Add ``compiled_sql`` as a template fields on ```ExecutionMode.WATCHER``` when using ``run_results.json`` by @pankajastro in #2070 * Set ``push_run_results_to_xcom`` kwargs correctly for invocation mode subprocess and Watcher mode by @pankajastro in #2067 * Store compiled SQL as template field for dbt callback events in ``ExecutionMode.WATCHER`` by @pankajkoti in #2068 * Add initial documentation for ``ExecutionMode.WATCHER`` by @tatiana in #2046 * Support running ``State.UPSTREAM_FAILED`` tasks when WATCHER consumer upstream tasks fail by @tatiana in #2062 * Fail sensor tasks immediately if the ``ExecutionMode.WATCHER`` producer task fails by @pankajastro in #2040 * Add ``WATCHER``` to GitHub issue template by @tatiana in #2056 * Add support for ``TestBehavior.AFTER_ALL`` with ``ExecutionMode.WATCHER`` by @pankajastro in #2049 * Add support for ``TestBehavior.NONE`` with ``ExecutionMode.WATCHER`` by @pankajastro in #2047 * Fix ``ExecutionMode.WATCHER`` behaviour with ``DbtTaskGroup`` by @tatiana in #2044 * Fix Cosmos behaviour when using watcher with ``InvocationMode.DBT_RUNNER`` by @tatiana in #2048 * Add Airflow 3 plugin for dbt docs with multiple dbt projects support by @pankajkoti in #2009, check the [documentation](https://astronomer.github.io/astronomer-cosmos/configuration/hosting-docs.html). * Initial support to ``dbt Fusion`` by @tatiana in #1803. More details [here](https://astronomer.github.io/astronomer-cosmos/configuration/dbt-fusion). * Support to prune sources without downstream references in dbt projects by @corsettigyg in #1988 * Allow to set task display name as a user-defined function by @corsettigyg in #1761 * Add dbt project's hash to dag docs to support dag versioning in Airflow 3 by @pankajkoti in #1907 * feat: Add Jinja templating support for ``dbt_cmd_flags`` by @skillicinski in #1899 * Add Scarf metric to collect the execution mode uses by @pankajastro in #1981 * Support Airflow 3.1 by @tatiana in #1980 * Add MySQL profile mapping by @Lee2532 in #1977 * Add sqlserver profile mapping by @pankajastro in #1737 **Enhancement** * Use XCom to store sql when using ``ExecutionMode.AIRFLOW_ASYNC`` by @pankajastro in #1934 * Refactor ``AIRFLOW_ASYNC`` teardown so it doesn't install the virtualenv by @pankajastro in #1938 * Reuse the virtual env for ``AIRFLOW_ASYNC`` setup task by @pankajastro in #1939 * Improve dataset/asset experience in Cosmos by @tatiana in #2030 * Add ``downstreams`` to ``DbtNode`` by @wornjs in #2028 **Bug fixes** * Fix tags extraction by @ms32035 in #1915 * Fix task flow operator args by @anyapriya in #2024 **Documentation** * Add documentation for Airflow 3 Plugin supporting dbt docs for multiple dbt projects by @pankajkoti in #2063 * Add Cosmos Deferrable Operator Guide by @pankajastro in #1922 * Add dbt Fusion documentation by @tatiana in #1824 #1830 * Update dbt-fusion.rst to explicitly highlight it is in alpha by @tatiana in #1838 * Fix a bunch of docs build errors and warnings by @pankajkoti in #1886 * Add docs note for param virtualenv_dir for async execution mode by @pankajastro in #1969 * Use pepy.tech downloads badge in README by @pankajkoti in #1920 * Correct the default value of ``cache_dir`` by @seokyun.ha in #2027 **Others** * Promote @corsettigyg to committer by @tatiana in #1985 * Add @pankajkoti and @pankajastro to ``contributors.rst`` by @tatiana in #1983 * Update setup script for airflow3 script by @dwreeves in #2023 * Prevent pytest from trying to test classes that aren't actually tests by @anyapriya in #2032 * Fix ``dag.test()`` for Airflow 3.1+ by syncing DAG to database bby @kaxil in #2037 * Disable Scarf in CI by @pankajastro in #2016 * Fix failing dbt Fusion tests when run in parallel in CI by @pankajkoti in #1896 * Fix MyPy issues related to ``ObjectStoragePath`` in main branch by @tatiana in #2012 * Cleanup example dbt event JSON dictionaries kept for XCOM referencby @pankajkoti in #1997 * Bump min hatch version that includes fixes for click>=8.3.0 by @pankajkoti in #1996 * Use official postgres image from Docker hub for kubernetes setup by @pankajkoti in #1986 * Use click<8.3.0 for hatch as click 8.3 breaks hatch by @pankajkoti in #1987 * Pin Airflow version in type check CI job by @pankajastro in #2003 * Improve comments after feedback on #1948 by @tatiana in #1963 * Fix running tests with dbt Fusion 2.0.0 preview versions by @tatiana in #1948 * Test hardening of dbt node having tags as unset or missing by @pankajkoti in #1918 * Fix Sphinx issue in the main branch by @tatiana in #2064 * pre-commit autoupdate in #2065, #2043, #2033, #2019, #1990, #2019, #2008, #1941, #1935, #1924 * GitHub dependabot update in #2051, #2050, #2038, #2022, #1947, #1955, #1946, #1944, #1945, #1928, #1921, #1917 Co-authored-by: Pankaj Koti <pankaj.koti@astronomer.io> Co-authored-by: Pankaj Singh <pankaj.singh@astronomer.io> Co-authored-by: Pankaj Koti <pankajkoti699@gmail.com>
The solution utilises forward references (string literals) for the
ObjectStoragePathtype, allowing mypy to understand the types without requiring the actual class to be available at runtime. This is the standard approach for handling optional dependencies in type annotations.Cosmos' main branch is red due to mypy checks failing, as observed in this sample GitHub Actions job:
https://github.com/astronomer/astronomer-cosmos/actions/runs/18122118077/job/51706664314
The same job was working without failures two days ago:
https://github.com/astronomer/astronomer-cosmos/actions/runs/18111649257/job/51539065565
Changes introduced
TYPE_CHECKINGimports: Added conditional imports forObjectStoragePathin all affected files using theTYPE_CHECKINGpattern to avoid runtime import errors when the class is not available.ObjectStoragePathin type annotations to avoid mypy errors:Fixed specific issues:
_configured_cache_dirvariable and_configure_remote_cache_dir()function return typemanifest_pathfield andmandatory_pathsdictionary type annotations_configure_remote_target_path()returntype and _construct_dest_file_path()parameter type_get_dbt_ls_remote_cache()parameter typeVerification
I used the same command as the CI to confirm that the introduced changes fix the original issue:
And I also confirmed in the CI:

https://github.com/astronomer/astronomer-cosmos/actions/runs/18167212785/job/51712529044?pr=2012