Fix/1550 reset airflow db tests #1554
Conversation
✅ Deploy Preview for dlt-hub-docs canceled.
|
There was a problem hiding this comment.
this looks good! I think all the issues come from a few things we do here and are not directly documented ie. testing with minimal deps.
- please read and fix the comments
- please resubmit this PR directly to this repo, do not use forks. you have contributor access now. such access allows your PRs to be run with access to all CI secrets
- also please do
make lintbefore submitting so linter will pass. for automatic formatting pls usemake format
| @@ -84,7 +84,7 @@ jobs: | |||
| # key: venv-${{ matrix.os }}-${{ matrix.python-version }}-${{ hashFiles('**/poetry.lock') }} | |||
|
|
|||
| - name: Install dependencies | |||
| run: poetry install --no-interaction --with sentry-sdk | |||
| run: poetry install --no-interaction --with sentry-sdk,airflow | |||
There was a problem hiding this comment.
please do not add this dependency here. the whole structure of test_common is such that we start with bare deps (sentry-sdk will go out soon as well) and then gradually increase the number of deps.
this is to catch any problems with people assuming certain dependencies exist while the bare library does not support them
in case of this PR we are changing the test code but still we should not affect how other tests are run
| @@ -115,4 +115,16 @@ def _create_pipeline_instance_id(self) -> str: | |||
| # disable httpx request logging (too verbose when testing qdrant) | |||
| logging.getLogger("httpx").setLevel("WARNING") | |||
|
|
|||
| # reset airflow db | |||
| from airflow.utils import db | |||
There was a problem hiding this comment.
allow for the import to fail so we do it only when airflow is installed
| logging.getLogger(log).setLevel("ERROR") | ||
|
|
||
| try: | ||
| db.resetdb() |
There was a problem hiding this comment.
could you take a look at the function below in tests/helpers/airflow_tests/utils.py
def setup_airflow() -> None:
which we use to initialize airflow for our tests. it for example disables loading examples. but maybe it is irrelevant for the simple reset. is there anything we should also do here?
There was a problem hiding this comment.
I don't think that we need to add anything.
Thanks for the comments and recommendations. I have openned a second PR with the proper branch: #1559 |
Description
Forces Airflow local database reset when running tests.
Related Issues