potiuk commented on PR #35160:
URL: https://github.com/apache/airflow/pull/35160#issuecomment-1788018589
BOOM ,, MERGED
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To u
potiuk merged PR #35160:
URL: https://github.com/apache/airflow/pull/35160
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@airflow.a
vincbeck commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1377940687
##
airflow/settings.py:
##
@@ -205,13 +205,33 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging",
"donot_modify_handlers", fallback=Fal
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1377923346
##
TESTING.rst:
##
@@ -72,56 +66,259 @@ fixture. This in turn makes Airflow load test
configuration from the file
defaults from ``airflow/config_templates/config.yml
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1377661892
##
TESTING.rst:
##
@@ -72,56 +66,259 @@ fixture. This in turn makes Airflow load test
configuration from the file
defaults from ``airflow/config_templates/config.yml
vincbeck commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1377648240
##
TESTING.rst:
##
@@ -72,56 +66,259 @@ fixture. This in turn makes Airflow load test
configuration from the file
defaults from ``airflow/config_templates/config.y
vincbeck commented on PR #35160:
URL: https://github.com/apache/airflow/pull/35160#issuecomment-1787284998
> > One thought. I am wondering if we should not be opinionated here
regarding pytest-xdist. In this PR you leave the choice to the user to use
pytest-xdist or not. But does it make se
potiuk commented on PR #35160:
URL: https://github.com/apache/airflow/pull/35160#issuecomment-1787234830
> One thought. I am wondering if we should not be opinionated here regarding
pytest-xdist. In this PR you leave the choice to the user to use pytest-xdist
or not. But does it make sense
potiuk commented on PR #35160:
URL: https://github.com/apache/airflow/pull/35160#issuecomment-1786157180
Check it now @vincbeck (also updated it in the docs).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL ab
potiuk commented on PR #35160:
URL: https://github.com/apache/airflow/pull/35160#issuecomment-1786093658
> One thought. I am wondering if we should not be opinionated here regarding
`pytest-xdist`. In this PR you leave the choice to the user to use
`pytest-xdist` or not. But does it make se
vincbeck commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1376782819
##
airflow/settings.py:
##
@@ -205,13 +205,31 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging",
"donot_modify_handlers", fallback=Fal
vincbeck commented on PR #35160:
URL: https://github.com/apache/airflow/pull/35160#issuecomment-1786005595
One thought. I am wondering if we should not be opinionated here regarding
`pytest-xdist`. In this PR you leave the choice to the user to use
`pytest-xdist` or not. But that it makes s
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1376539399
##
tests/system/providers/amazon/aws/utils/__init__.py:
##
@@ -106,6 +106,8 @@ def _fetch_from_ssm(key: str, test_name: str | None = None)
-> str:
# Since a defa
vincbeck commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1376570794
##
tests/system/providers/amazon/aws/utils/__init__.py:
##
@@ -106,6 +106,8 @@ def _fetch_from_ssm(key: str, test_name: str | None = None)
-> str:
# Since a de
potiuk commented on PR #35160:
URL: https://github.com/apache/airflow/pull/35160#issuecomment-1785668129
This is nice - after the rebase we got this new tests failing in "Non-DB"
test case:
![image](https://github.com/apache/airflow/assets/595491/353aa1e7-e702-4eac-89e9-17d794cb81a7)
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1376539399
##
tests/system/providers/amazon/aws/utils/__init__.py:
##
@@ -106,6 +106,8 @@ def _fetch_from_ssm(key: str, test_name: str | None = None)
-> str:
# Since a defa
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1376539399
##
tests/system/providers/amazon/aws/utils/__init__.py:
##
@@ -106,6 +106,8 @@ def _fetch_from_ssm(key: str, test_name: str | None = None)
-> str:
# Since a defa
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1376539399
##
tests/system/providers/amazon/aws/utils/__init__.py:
##
@@ -106,6 +106,8 @@ def _fetch_from_ssm(key: str, test_name: str | None = None)
-> str:
# Since a defa
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1376495329
##
tests/operators/test_python.py:
##
@@ -844,17 +848,30 @@ def f(exit_code):
assert ti.state == expected_state
+venv_cache_path = tempfile.mkdtemp(pr
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1376495329
##
tests/operators/test_python.py:
##
@@ -844,17 +848,30 @@ def f(exit_code):
assert ti.state == expected_state
+venv_cache_path = tempfile.mkdtemp(pr
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1376495329
##
tests/operators/test_python.py:
##
@@ -844,17 +848,30 @@ def f(exit_code):
assert ti.state == expected_state
+venv_cache_path = tempfile.mkdtemp(pr
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1376495329
##
tests/operators/test_python.py:
##
@@ -844,17 +848,30 @@ def f(exit_code):
assert ti.state == expected_state
+venv_cache_path = tempfile.mkdtemp(pr
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1376495329
##
tests/operators/test_python.py:
##
@@ -844,17 +848,30 @@ def f(exit_code):
assert ti.state == expected_state
+venv_cache_path = tempfile.mkdtemp(pr
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1376495329
##
tests/operators/test_python.py:
##
@@ -844,17 +848,30 @@ def f(exit_code):
assert ti.state == expected_state
+venv_cache_path = tempfile.mkdtemp(pr
potiuk commented on PR #35160:
URL: https://github.com/apache/airflow/pull/35160#issuecomment-1785527718
OK - after merging the "db_test" marker change - this change shoould be
**actually** reviewable (even if quite big - but I can't think of anything else
I can split out from it to make it
jens-scheffler-bosch commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1376289820
##
tests/operators/test_python.py:
##
@@ -844,17 +848,30 @@ def f(exit_code):
assert ti.state == expected_state
+venv_cache_path = tempf
vincbeck commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1376268974
##
tests/system/providers/amazon/aws/utils/__init__.py:
##
@@ -106,6 +106,8 @@ def _fetch_from_ssm(key: str, test_name: str | None = None)
-> str:
# Since a de
potiuk commented on PR #35160:
URL: https://github.com/apache/airflow/pull/35160#issuecomment-1784629964
Extracted out https://github.com/apache/airflow/pull/35264 - with purely
adding `db_test` marker. Splitting that change into two should make it far
easier to review.
--
This is an aut
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375559213
##
tests/operators/test_python.py:
##
@@ -844,17 +848,30 @@ def f(exit_code):
assert ti.state == expected_state
+venv_cache_path = tempfile.mkdtemp(pr
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375550277
##
tests/operators/test_python.py:
##
@@ -844,17 +848,30 @@ def f(exit_code):
assert ti.state == expected_state
+venv_cache_path = tempfile.mkdtemp(pr
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375550277
##
tests/operators/test_python.py:
##
@@ -844,17 +848,30 @@ def f(exit_code):
assert ti.state == expected_state
+venv_cache_path = tempfile.mkdtemp(pr
jens-scheffler-bosch commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375538516
##
tests/operators/test_python.py:
##
@@ -844,17 +848,30 @@ def f(exit_code):
assert ti.state == expected_state
+venv_cache_path = tempf
potiuk commented on PR #35160:
URL: https://github.com/apache/airflow/pull/35160#issuecomment-1784206337
Just MSSQL is a bit unstable - that has not changed :(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL ab
potiuk commented on PR #35160:
URL: https://github.com/apache/airflow/pull/35160#issuecomment-1784196280
Good numbers also for Public runners:
We had minimum on average (excluding MSSQL which is a bit longer) - 5x 35
minutes for "core change" PR on Public runers = 175 build mintes .
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375477256
##
tests/jobs/test_triggerer_job.py:
##
@@ -110,6 +112,9 @@ def create_trigger_in_db(session, trigger, operator=None):
return dag_model, run, trigger_orm, task_in
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375471350
##
tests/jobs/test_triggerer_job.py:
##
@@ -110,6 +112,9 @@ def create_trigger_in_db(session, trigger, operator=None):
return dag_model, run, trigger_orm, task_in
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375471350
##
tests/jobs/test_triggerer_job.py:
##
@@ -110,6 +112,9 @@ def create_trigger_in_db(session, trigger, operator=None):
return dag_model, run, trigger_orm, task_in
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375471350
##
tests/jobs/test_triggerer_job.py:
##
@@ -110,6 +112,9 @@ def create_trigger_in_db(session, trigger, operator=None):
return dag_model, run, trigger_orm, task_in
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375471350
##
tests/jobs/test_triggerer_job.py:
##
@@ -110,6 +112,9 @@ def create_trigger_in_db(session, trigger, operator=None):
return dag_model, run, trigger_orm, task_in
hussein-awala commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375469882
##
tests/jobs/test_triggerer_job.py:
##
@@ -110,6 +112,9 @@ def create_trigger_in_db(session, trigger, operator=None):
return dag_model, run, trigger_orm,
potiuk commented on PR #35160:
URL: https://github.com/apache/airflow/pull/35160#issuecomment-1784152557
I got the first "fully green pass" (Quarantined tests are green but really
"skipped" so I will need to fix that one).
Looking for more reviews while adding some best practices and
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375463230
##
tests/jobs/test_triggerer_job.py:
##
@@ -110,6 +112,9 @@ def create_trigger_in_db(session, trigger, operator=None):
return dag_model, run, trigger_orm, task_in
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375462594
##
tests/operators/test_python.py:
##
@@ -844,17 +848,30 @@ def f(exit_code):
assert ti.state == expected_state
+venv_cache_path = tempfile.mkdtemp(pr
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375456975
##
tests/www/views/test_views_rendered.py:
##
@@ -250,6 +254,13 @@ def test_rendered_template_secret(admin_client,
create_dag_run, task_secret):
assert ti.state
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375456699
##
tests/system/providers/amazon/aws/utils/__init__.py:
##
@@ -106,6 +106,8 @@ def _fetch_from_ssm(key: str, test_name: str | None = None)
-> str:
# Since a defa
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375456249
##
tests/providers/amazon/aws/operators/test_glue.py:
##
@@ -38,10 +37,7 @@
class TestGlueJobOperator:
-@pytest.fixture(autouse=True)
-def setup_method(se
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375455982
##
tests/conftest.py:
##
@@ -167,36 +195,66 @@ def pytest_addoption(parser):
group.addoption(
"--integration",
action="append",
+dest="i
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375455786
##
dev/breeze/src/airflow_breeze/utils/run_tests.py:
##
@@ -87,3 +89,262 @@ def run_docker_compose_tests(
def file_name_from_test_type(test_type: str):
test_type
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375454641
##
dev/breeze/src/airflow_breeze/commands/testing_commands.py:
##
@@ -431,12 +487,34 @@ def command_for_tests(
collect_only=collect_only,
remove_arm_
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375454641
##
dev/breeze/src/airflow_breeze/commands/testing_commands.py:
##
@@ -431,12 +487,34 @@ def command_for_tests(
collect_only=collect_only,
remove_arm_
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375454311
##
dev/breeze/src/airflow_breeze/commands/testing_commands.py:
##
@@ -180,6 +190,21 @@ def _run_test(
"--rm",
"airflow",
]
+run_cmd.extend(
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1375453946
##
airflow/configuration.py:
##
@@ -2062,19 +2062,6 @@ def make_group_other_inaccessible(file_path: str):
)
-# Historical convenience functions to access
potiuk commented on PR #35160:
URL: https://github.com/apache/airflow/pull/35160#issuecomment-1781422687
(addressed all comments @vincbeck - still working on getting it all green
but with the feedback it already got a bit better).
--
This is an automated message from the Apache Git Servic
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373385808
##
dev/breeze/src/airflow_breeze/global_constants.py:
##
@@ -44,7 +44,7 @@
ALLOWED_PYTHON_MAJOR_MINOR_VERSIONS = ["3.8", "3.9", "3.10", "3.11"]
DEFAULT_PYTHON_MAJOR_
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373384808
##
dev/breeze/src/airflow_breeze/commands/testing_commands.py:
##
@@ -250,6 +276,35 @@ def _run_tests_in_pool(
skip_docker_compose_down: bool,
):
escaped_te
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373345164
##
.github/workflows/ci.yml:
##
@@ -1027,7 +1025,7 @@ jobs:
uses: ./.github/actions/migration_tests
- name: >
Tests:
${{matrix.python-versi
vincbeck commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373274314
##
dev/breeze/src/airflow_breeze/commands/testing_commands.py:
##
@@ -250,6 +276,35 @@ def _run_tests_in_pool(
skip_docker_compose_down: bool,
):
escaped_
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373305830
##
tests/providers/amazon/aws/utils/test_connection_wrapper.py:
##
@@ -27,14 +28,19 @@
from airflow.models import Connection
from airflow.providers.amazon.aws.utils.
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373276572
##
tests/providers/apache/druid/hooks/test_druid.py:
##
@@ -26,7 +26,8 @@
from airflow.providers.apache.druid.hooks.druid import DruidDbApiHook,
DruidHook, Ingestion
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373276572
##
tests/providers/apache/druid/hooks/test_druid.py:
##
@@ -26,7 +26,8 @@
from airflow.providers.apache.druid.hooks.druid import DruidDbApiHook,
DruidHook, Ingestion
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373274323
##
tests/providers/amazon/aws/hooks/test_batch_waiters.py:
##
@@ -241,7 +241,7 @@ def describe_jobs_response(job_id: str = "mock-job-id",
status: str = INTERMEDIA
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373270353
##
tests/providers/amazon/aws/hooks/test_base_aws.py:
##
@@ -590,40 +590,40 @@ def import_mock(name, *args):
[mock.call.get_default_id_token_credentials(
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373270353
##
tests/providers/amazon/aws/hooks/test_base_aws.py:
##
@@ -590,40 +590,40 @@ def import_mock(name, *args):
[mock.call.get_default_id_token_credentials(
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373266881
##
tests/callbacks/test_callback_requests.py:
##
@@ -32,11 +32,7 @@
from airflow.utils import timezone
from airflow.utils.state import State
-TI = TaskInstance(
R
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373264647
##
tests/api_connexion/endpoints/test_dag_run_endpoint.py:
##
@@ -599,12 +601,12 @@ class TestGetDagRunsEndDateFilters(TestDagRunEndpoint):
[
(
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373261758
##
setup.py:
##
@@ -310,7 +310,7 @@ def write_version(filename: str = str(AIRFLOW_SOURCES_ROOT
/ "airflow" / "git_ve
# By adding a lot of whitespace separation.
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373260407
##
setup.cfg:
##
@@ -101,7 +101,7 @@ install_requires =
flask-session>=0.4.0
flask-wtf>=0.15
google-re2>=1.0
-graphviz>=0.12
+#graphviz>=0.12
R
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373260982
##
setup.cfg:
##
@@ -101,7 +101,7 @@ install_requires =
flask-session>=0.4.0
flask-wtf>=0.15
google-re2>=1.0
-graphviz>=0.12
+#graphviz>=0.12
R
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373258762
##
scripts/ci/docker-compose/backend-none.yml:
##
@@ -15,19 +14,11 @@
# KIND, either express or implied. See the License for the
# specific language governing permi
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373253135
##
Dockerfile.ci:
##
@@ -1020,273 +1019,31 @@ if [[ ${DOWNGRADE_SQLALCHEMY=} == "true" ]]; then
pip check
fi
+pip uninstall --root-user-action ignore "pytest-c
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373249661
##
Dockerfile.ci:
##
@@ -1020,273 +1019,31 @@ if [[ ${DOWNGRADE_SQLALCHEMY=} == "true" ]]; then
pip check
fi
+pip uninstall --root-user-action ignore "pytest-c
potiuk commented on code in PR #35160:
URL: https://github.com/apache/airflow/pull/35160#discussion_r1373249661
##
Dockerfile.ci:
##
@@ -1020,273 +1019,31 @@ if [[ ${DOWNGRADE_SQLALCHEMY=} == "true" ]]; then
pip check
fi
+pip uninstall --root-user-action ignore "pytest-c
72 matches
Mail list logo