Fix test_integration_virtualenv_operator#1718
Conversation
✅ Deploy Preview for sunny-pastelito-5ecb04 canceled.
|
Deploying astronomer-cosmos with
|
| Latest commit: |
ab82177
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e3b01ecc.astronomer-cosmos.pages.dev |
| Branch Preview URL: | https://fix-1707.astronomer-cosmos.pages.dev |
Remove template
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1718 +/- ##
=======================================
Coverage 97.50% 97.50%
=======================================
Files 83 83
Lines 5049 5049
=======================================
Hits 4923 4923
Misses 126 126 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| ) | ||
|
|
||
| with DAG("example_virtualenv_mini", start_date=datetime(2022, 1, 1)) as dag: | ||
| pre = BashOperator(task_id="pre", bash_command="mkdir -p /tmp/persistent-venv2") |
There was a problem hiding this comment.
I know this is still is draft, but taking an early review. I don't think this could be the right approach. Ideally DbtSeedVirtualenvOperator itself should create this path.
There was a problem hiding this comment.
also, what if we try "/tmp/persistent-venv2" instead of Path("/tmp/persistent-venv2") in the argument value for the operator? does it have an effect in case of Airflow 3 and help with the rendering and at the same the operator execution is not impacted?
We can reflect that in our docs that the type needs to be a string and not Path.
There was a problem hiding this comment.
I think from a rendering point of view it should work, but type for virtualenv_dir is Path or None
There was a problem hiding this comment.
There was a problem hiding this comment.
yes, the CI was unhappy, but I pushed a commit 1f1947c that seems to help
|
As discussed in https://github.com/astronomer/astronomer-cosmos/pull/1718/files#r2065555001, I agree that we should implement the solution as part of the operator itself, rather than expecting end-users to create a separate task to create the directory. The other PR #1721 looks more promising, even though it still needs more work. I'm therefore closing this PR for now. |
closes: #1707
For AF3, it appears that the render_template is attempting to resolve the virtualenv_dir templated field path, which leads to an error during execution.
This PR introduces a pre-task step to generate the virtualenv_dir value before it's used in the template rendering process, preventing the resolution error.