-
Notifications
You must be signed in to change notification settings - Fork 180
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 Azure Container Instance as Execution Mode #771
Conversation
👷 Deploy Preview for amazing-pothos-a3bca0 processing.
|
Note: I haven't added the step-by-step documentation yet. This is deliberate, as it'd be great to first get feedback on the implementation so I can make sure to align implementation with docs. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #771 +/- ##
==========================================
- Coverage 94.81% 94.40% -0.42%
==========================================
Files 55 56 +1
Lines 2470 2520 +50
==========================================
+ Hits 2342 2379 +37
- Misses 128 141 +13 ☔ View full report in Codecov by Sentry. |
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.
Thanks for your contribution, this looks awesome! Minor comments below from me, I'm not familiar with the ACI, so it might be good to have another set of 👀 here for review.
You'll have to add the new operator imports to the init.py file below following like what's done for Docker and Kubernetes:
astronomer-cosmos/cosmos/__init__.py
Lines 34 to 52 in b58b018
try: | |
from cosmos.operators.docker import ( | |
DbtLSDockerOperator, | |
DbtRunDockerOperator, | |
DbtRunOperationDockerOperator, | |
DbtSeedDockerOperator, | |
DbtSnapshotDockerOperator, | |
DbtTestDockerOperator, | |
) | |
except ImportError: | |
DbtLSDockerOperator = MissingPackage("cosmos.operators.docker.DbtLSDockerOperator", "docker") | |
DbtRunDockerOperator = MissingPackage("cosmos.operators.docker.DbtRunDockerOperator", "docker") | |
DbtRunOperationDockerOperator = MissingPackage( | |
"cosmos.operators.docker.DbtRunOperationDockerOperator", | |
"docker", | |
) | |
DbtSeedDockerOperator = MissingPackage("cosmos.operators.docker.DbtSeedDockerOperator", "docker") | |
DbtSnapshotDockerOperator = MissingPackage("cosmos.operators.docker.DbtSnapshotDockerOperator", "docker") | |
DbtTestDockerOperator = MissingPackage("cosmos.operators.docker.DbtTestDockerOperator", "docker") |
Thanks for addressing all the comments! This looks great to me. I'd like to get another person from the team to review for adding in a new execution mode. Feel free to add in the step-by-step instructions in |
@jbandoro I've now added a step-by-step-guide. It includes a reference to an example I've put together in the cosmos-example repo. I haven't opened the PR for it yet, because I first fixed the existing examples in there for which a PR is now open here. Once that one is merged, I can open the one with the example DAG for Azure Container Instance. |
I noticed in #771 that there was a lot of repeated class constructors in order to add a new execution mode that is common among `local`, `docker` and `kubernetes` and there is no test coverage for the constructors and methods in some of the operators. This PR attempts to make it easier to add new execution operators in the future. ## Breaking Change? None There may be task UI color differences with the kuberentes/docker operators, since now all of LS/Seed/Run etc. operators across execution modes have the same task colors.
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.
Hi @danielvdende - this feature is very handy for users running in Azure. Thanks for working on this!
We recently merged @jbandoro 's #774 with many improvements to the existing operators/execution modes. Please, could you rebase and follow the new pattern? It will improve our code maintainability :)
@danielvdende we're trying to get an alpha release of v1.4 out soon, and I was hoping to get this work merged, but the unit tests are failing after merging in the latest commits from main. I checked out this branch and can't reproduce the failing unit tests locally. When I find a solution I'll open up a PR in your fork so that we can we can merge it after all the tests pass, apologies that this is taking some time. edit: @danielvdende I was able to get the tests working in this PR in your fork: https://github.com/danielvdende/astronomer-cosmos/pull/1/files |
Fix broken tests in dvde/add-aci-support
Great @jbandoro! I just merged the PR, and see pipelines running here again now too. |
Features * Add dbt docs natively in Airflow via plugin by @dwreeves in #737 * Add support for ``InvocationMode.DBT_RUNNER`` for local execution mode by @jbandoro in #850 * Support partial parsing to render DAGs faster when using ``ExecutionMode.LOCAL``, ``ExecutionMode.VIRTUALENV`` and ``LoadMode.DBT_LS`` by @dwreeves in #800 * Add Azure Container Instance as Execution Mode by @danielvdende in #771 * Add dbt build operators by @dylanharper-qz in #795 * Add dbt profile config variables to mapped profile by @ykuc in #794 * Add more template fields to ``DbtBaseOperator`` by @dwreeves in #786 Bug fixes * Make ``PostgresUserPasswordProfileMapping`` schema argument optional by @FouziaTariq in #683 * Fix ``folder_dir`` not showing on logs for ``DbtDocsS3LocalOperator`` by @PrimOox in #856 * Improve ``dbt ls`` parsing resilience to missing tags/config by @tatiana in #859 * Fix ``operator_args`` modified in place in Airflow converter by @jbandoro in #835 * Fix Docker and Kubernetes operators execute method resolution by @jbandoro in #849 Docs * Fix docs homepage link by @jlaneve in #860 * Fix docs ``ExecutionConfig.dbt_project_path`` by @jbandoro in #847 * Fix typo in MWAA getting started guide by @jlaneve in #846 Others * Add performance integration tests by @jlaneve in #827 * Add ``connect_retries`` to databricks profile to fix expensive integration failures by @jbandoro in #826 * Add import sorting (isort) to Cosmos by @jbandoro in #866 * Add Python 3.11 to CI/tests by @tatiana and @jbandoro in #821, #824 and #825 * Fix failing ``test_created_pod`` for ``apache-airflow-providers-cncf-kubernetes`` after v8.0.0 update by @jbandoro in #854 * Extend ``DatabricksTokenProfileMapping`` test to include session properties by @tatiana in #858 * Fix broken integration test uncovered from Pytest 8.0 update by @jbandoro in #845 * Pre-commit hook updates in #834, #843 and #852
Features * Add dbt docs natively in Airflow via plugin by @dwreeves in #737 * Add support for ``InvocationMode.DBT_RUNNER`` for local execution mode by @jbandoro in #850 * Support partial parsing to render DAGs faster when using ``ExecutionMode.LOCAL``, ``ExecutionMode.VIRTUALENV`` and ``LoadMode.DBT_LS`` by @dwreeves in #800 * Improve performance by 22-35% or more by caching partial parse artefact by @tatiana in #904 * Add Azure Container Instance as Execution Mode by @danielvdende in #771 * Add dbt build operators by @dylanharper-qz in #795 * Add dbt profile config variables to mapped profile by @ykuc in #794 * Add more template fields to ``DbtBaseOperator`` by @dwreeves in #786 * Add ``pip_install_options`` argument to operators by @octiva in #808 Bug fixes * Make ``PostgresUserPasswordProfileMapping`` schema argument optional by @FouziaTariq in #683 * Fix ``folder_dir`` not showing on logs for ``DbtDocsS3LocalOperator`` by @PrimOox in #856 * Improve ``dbt ls`` parsing resilience to missing tags/config by @tatiana in #859 * Fix ``operator_args`` modified in place in Airflow converter by @jbandoro in #835 * Fix Docker and Kubernetes operators execute method resolution by @jbandoro in #849 * Fix ``TrinoBaseProfileMapping`` required parameter for non method authentication by @AlexandrKhabarov in #921 * Fix global flags for lists by @ms32035 in #863 * Fix ``GoogleCloudServiceAccountDictProfileMapping`` when getting values from the Airflow connection ``extra__`` keys by @glebkrapivin in #923 * Fix using the dag as a keyword argument as ``specific_args_keys`` in DbtTaskGroup by @tboutaour in #916 * Fix ACI integration (``DbtAzureContainerInstanceBaseOperator``) by @danielvdende in #872 * Fix setting dbt project dir to the tmp dir by @dwreeves in #873 * Fix dbt docs operator to not use ``graph.gpickle`` file when ``--no-write-json`` is passed by @dwreeves in #883 * Make Pydantic a required dependency by @pankajkoti in #939 * Gracefully error if users try to ``emit_datasets`` with ``Airflow 2.9.0`` or ``2.9.1`` by @tatiana in #948 * Fix parsing tests that have no parents in #933 by @jlaneve * Correct ``root_path`` in partial parse cache by @pankajkoti in #950 Docs * Fix docs homepage link by @jlaneve in #860 * Fix docs ``ExecutionConfig.dbt_project_path`` by @jbandoro in #847 * Fix typo in MWAA getting started guide by @jlaneve in #846 * Fix typo related to exporting docs to GCS by @tboutaour in #922 * Improve partial parsing docs by @tatiana in #898 * Improve docs for datasets for airflow >= 2.4 by @SiddiqueAhmad in #879 * Improve test behaviour docs to highlight ``warning`` feature in the ``virtualenv`` mode by @mc51 in #910 * Fix docs typo by @SiddiqueAhmad in #917 * Improve Astro docs by @RNHTTR in #951 Others * Add performance integration tests by @jlaneve in #827 * Enable ``append_env`` in ``operator_args`` by default by @tatiana in #899 * Change default ``append_env`` behaviour depending on Cosmos ``ExecutionMode`` by @pankajkoti and @pankajastro in #954 * Expose the ``dbt`` graph in the ``DbtToAirflowConverter`` class by @tommyjxl in #886 * Improve dbt docs plugin rendering padding by @dwreeves in #876 * Add ``connect_retries`` to databricks profile to fix expensive integration failures by @jbandoro in #826 * Add import sorting (isort) to Cosmos by @jbandoro in #866 * Add Python 3.11 to CI/tests by @tatiana and @jbandoro in #821, #824 and #825 * Fix failing ``test_created_pod`` for ``apache-airflow-providers-cncf-kubernetes`` after v8.0.0 update by @jbandoro in #854 * Extend ``DatabricksTokenProfileMapping`` test to include session properties by @tatiana in #858 * Fix broken integration test uncovered from Pytest 8.0 update by @jbandoro in #845 * Add Apache Airflow 2.9 to the test matrix by @tatiana in #940 * Replace deprecated ``DummyOperator`` by ``EmptyOperator`` if Airflow >=2.4.0 by @tatiana in #900 * Improve logs to troubleshoot issue in 1.4.0a2 with astro-cli by @tatiana in #947 * Fix issue when publishing a new release to PyPI by @tatiana in #946 * Pre-commit hook updates in #820, #834, #843 and #852, #890, #896, #901, #905, #908, #919, #931, #941
) I noticed in astronomer#771 that there was a lot of repeated class constructors in order to add a new execution mode that is common among `local`, `docker` and `kubernetes` and there is no test coverage for the constructors and methods in some of the operators. This PR attempts to make it easier to add new execution operators in the future. ## Breaking Change? None There may be task UI color differences with the kuberentes/docker operators, since now all of LS/Seed/Run etc. operators across execution modes have the same task colors.
Features * Add dbt docs natively in Airflow via plugin by @dwreeves in astronomer#737 * Add support for ``InvocationMode.DBT_RUNNER`` for local execution mode by @jbandoro in astronomer#850 * Support partial parsing to render DAGs faster when using ``ExecutionMode.LOCAL``, ``ExecutionMode.VIRTUALENV`` and ``LoadMode.DBT_LS`` by @dwreeves in astronomer#800 * Add Azure Container Instance as Execution Mode by @danielvdende in astronomer#771 * Add dbt build operators by @dylanharper-qz in astronomer#795 * Add dbt profile config variables to mapped profile by @ykuc in astronomer#794 * Add more template fields to ``DbtBaseOperator`` by @dwreeves in astronomer#786 Bug fixes * Make ``PostgresUserPasswordProfileMapping`` schema argument optional by @FouziaTariq in astronomer#683 * Fix ``folder_dir`` not showing on logs for ``DbtDocsS3LocalOperator`` by @PrimOox in astronomer#856 * Improve ``dbt ls`` parsing resilience to missing tags/config by @tatiana in astronomer#859 * Fix ``operator_args`` modified in place in Airflow converter by @jbandoro in astronomer#835 * Fix Docker and Kubernetes operators execute method resolution by @jbandoro in astronomer#849 Docs * Fix docs homepage link by @jlaneve in astronomer#860 * Fix docs ``ExecutionConfig.dbt_project_path`` by @jbandoro in astronomer#847 * Fix typo in MWAA getting started guide by @jlaneve in astronomer#846 Others * Add performance integration tests by @jlaneve in astronomer#827 * Add ``connect_retries`` to databricks profile to fix expensive integration failures by @jbandoro in astronomer#826 * Add import sorting (isort) to Cosmos by @jbandoro in astronomer#866 * Add Python 3.11 to CI/tests by @tatiana and @jbandoro in astronomer#821, astronomer#824 and astronomer#825 * Fix failing ``test_created_pod`` for ``apache-airflow-providers-cncf-kubernetes`` after v8.0.0 update by @jbandoro in astronomer#854 * Extend ``DatabricksTokenProfileMapping`` test to include session properties by @tatiana in astronomer#858 * Fix broken integration test uncovered from Pytest 8.0 update by @jbandoro in astronomer#845 * Pre-commit hook updates in astronomer#834, astronomer#843 and astronomer#852
Features * Add dbt docs natively in Airflow via plugin by @dwreeves in astronomer#737 * Add support for ``InvocationMode.DBT_RUNNER`` for local execution mode by @jbandoro in astronomer#850 * Support partial parsing to render DAGs faster when using ``ExecutionMode.LOCAL``, ``ExecutionMode.VIRTUALENV`` and ``LoadMode.DBT_LS`` by @dwreeves in astronomer#800 * Improve performance by 22-35% or more by caching partial parse artefact by @tatiana in astronomer#904 * Add Azure Container Instance as Execution Mode by @danielvdende in astronomer#771 * Add dbt build operators by @dylanharper-qz in astronomer#795 * Add dbt profile config variables to mapped profile by @ykuc in astronomer#794 * Add more template fields to ``DbtBaseOperator`` by @dwreeves in astronomer#786 * Add ``pip_install_options`` argument to operators by @octiva in astronomer#808 Bug fixes * Make ``PostgresUserPasswordProfileMapping`` schema argument optional by @FouziaTariq in astronomer#683 * Fix ``folder_dir`` not showing on logs for ``DbtDocsS3LocalOperator`` by @PrimOox in astronomer#856 * Improve ``dbt ls`` parsing resilience to missing tags/config by @tatiana in astronomer#859 * Fix ``operator_args`` modified in place in Airflow converter by @jbandoro in astronomer#835 * Fix Docker and Kubernetes operators execute method resolution by @jbandoro in astronomer#849 * Fix ``TrinoBaseProfileMapping`` required parameter for non method authentication by @AlexandrKhabarov in astronomer#921 * Fix global flags for lists by @ms32035 in astronomer#863 * Fix ``GoogleCloudServiceAccountDictProfileMapping`` when getting values from the Airflow connection ``extra__`` keys by @glebkrapivin in astronomer#923 * Fix using the dag as a keyword argument as ``specific_args_keys`` in DbtTaskGroup by @tboutaour in astronomer#916 * Fix ACI integration (``DbtAzureContainerInstanceBaseOperator``) by @danielvdende in astronomer#872 * Fix setting dbt project dir to the tmp dir by @dwreeves in astronomer#873 * Fix dbt docs operator to not use ``graph.gpickle`` file when ``--no-write-json`` is passed by @dwreeves in astronomer#883 * Make Pydantic a required dependency by @pankajkoti in astronomer#939 * Gracefully error if users try to ``emit_datasets`` with ``Airflow 2.9.0`` or ``2.9.1`` by @tatiana in astronomer#948 * Fix parsing tests that have no parents in astronomer#933 by @jlaneve * Correct ``root_path`` in partial parse cache by @pankajkoti in astronomer#950 Docs * Fix docs homepage link by @jlaneve in astronomer#860 * Fix docs ``ExecutionConfig.dbt_project_path`` by @jbandoro in astronomer#847 * Fix typo in MWAA getting started guide by @jlaneve in astronomer#846 * Fix typo related to exporting docs to GCS by @tboutaour in astronomer#922 * Improve partial parsing docs by @tatiana in astronomer#898 * Improve docs for datasets for airflow >= 2.4 by @SiddiqueAhmad in astronomer#879 * Improve test behaviour docs to highlight ``warning`` feature in the ``virtualenv`` mode by @mc51 in astronomer#910 * Fix docs typo by @SiddiqueAhmad in astronomer#917 * Improve Astro docs by @RNHTTR in astronomer#951 Others * Add performance integration tests by @jlaneve in astronomer#827 * Enable ``append_env`` in ``operator_args`` by default by @tatiana in astronomer#899 * Change default ``append_env`` behaviour depending on Cosmos ``ExecutionMode`` by @pankajkoti and @pankajastro in astronomer#954 * Expose the ``dbt`` graph in the ``DbtToAirflowConverter`` class by @tommyjxl in astronomer#886 * Improve dbt docs plugin rendering padding by @dwreeves in astronomer#876 * Add ``connect_retries`` to databricks profile to fix expensive integration failures by @jbandoro in astronomer#826 * Add import sorting (isort) to Cosmos by @jbandoro in astronomer#866 * Add Python 3.11 to CI/tests by @tatiana and @jbandoro in astronomer#821, astronomer#824 and astronomer#825 * Fix failing ``test_created_pod`` for ``apache-airflow-providers-cncf-kubernetes`` after v8.0.0 update by @jbandoro in astronomer#854 * Extend ``DatabricksTokenProfileMapping`` test to include session properties by @tatiana in astronomer#858 * Fix broken integration test uncovered from Pytest 8.0 update by @jbandoro in astronomer#845 * Add Apache Airflow 2.9 to the test matrix by @tatiana in astronomer#940 * Replace deprecated ``DummyOperator`` by ``EmptyOperator`` if Airflow >=2.4.0 by @tatiana in astronomer#900 * Improve logs to troubleshoot issue in 1.4.0a2 with astro-cli by @tatiana in astronomer#947 * Fix issue when publishing a new release to PyPI by @tatiana in astronomer#946 * Pre-commit hook updates in astronomer#820, astronomer#834, astronomer#843 and astronomer#852, astronomer#890, astronomer#896, astronomer#901, astronomer#905, astronomer#908, astronomer#919, astronomer#931, astronomer#941
Description
This PR adds support for Azure Container Instance as an Execution Mode. In this mode, a container is run on Azure to execute the dbt commands.
Related Issue(s)
This closes #635
Checklist