Skip to content
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

Fix test_example_dags #32714

Merged
merged 2 commits into from
Aug 4, 2023
Merged

Conversation

ahidalgob
Copy link
Contributor

@ahidalgob ahidalgob commented Jul 20, 2023

This PR solves a couple of bugs in test_example_dags.py.

By going up to parents[3] we were going outside the repository root, luckily (or unluckily) the repo folder is also named airflow so the pattern airflow/**/example_dags/example_*.py still worked, but tests/system/providers/**/example_*.py wasn't used.

before: collected 105 items
after: collected 695 items

This discovered 2 new errors:

  • example_local_to_wasb.py was trivial to fix

  • example_redis_publish.py: this one fails because RedisPubSubSensor constructor calls Redis.pubsub().subscribe(), which just hangs and DagBag fails with timeout. For now I'm just deleting this operator from the example.

@ahidalgob
Copy link
Contributor Author

I think we should move the subscribe() call from RedisPubSubSensor constructor to the poke method. I'm happy to create another PR with this change.

@ahidalgob ahidalgob force-pushed the fix-test-example-dags branch 2 times, most recently from dddf19e to c3e1dfe Compare July 26, 2023 13:19
@ahidalgob ahidalgob requested a review from uranusjr July 26, 2023 13:20
By going up to `parents[3]` we were going outside the repository root,
luckily(or unluckily the repo folder is also named `airflow` so the
pattern `airflow/**/example_dags/example_*.py` still worked,
but `tests/system/providers/**/example_*.py` wasn't being used.

This discovered 2 new errors:

- `example_local_to_wasb.py` was trivial to fix

- `example_redis_publish.py`is more interesting: this one fails because
`RedisPubSubSensor` constructor calls Redis.pubsub().subscribe(), which
just hangs and DagBag fails with timeout. For now I'm just deleting this
operator from the example.
@ahidalgob ahidalgob force-pushed the fix-test-example-dags branch from c3e1dfe to 28e6b21 Compare July 27, 2023 08:06
@josh-fell
Copy link
Contributor

@ahidalgob Looks like there is a conflict to resolve. Can you take a look when you have a chance please?

@josh-fell josh-fell merged commit c048bd5 into apache:main Aug 4, 2023
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Aug 27, 2023
@ephraimbuddy ephraimbuddy added this to the Airflow 2.7.1 milestone Aug 27, 2023
ephraimbuddy pushed a commit that referenced this pull request Aug 28, 2023
By going up to `parents[3]` we were going outside the repository root,
luckily(or unluckily the repo folder is also named `airflow` so the
pattern `airflow/**/example_dags/example_*.py` still worked,
but `tests/system/providers/**/example_*.py` wasn't being used.

This discovered 2 new errors:

- `example_local_to_wasb.py` was trivial to fix

- `example_redis_publish.py`is more interesting: this one fails because
`RedisPubSubSensor` constructor calls Redis.pubsub().subscribe(), which
just hangs and DagBag fails with timeout. For now I'm just deleting this
operator from the example.

(cherry picked from commit c048bd5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers area:system-tests changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) provider:microsoft-azure Azure-related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants