You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "dstandish (via GitHub)" <gi...@apache.org> on 2024/04/09 21:46:30 UTC
[PR] Fix test side effects from TracebackSession [airflow]
dstandish opened a new pull request, #38885:
URL: https://github.com/apache/airflow/pull/38885
(no comment)
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix test side effects from TracebackSession [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish merged PR #38885:
URL: https://github.com/apache/airflow/pull/38885
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix test side effects from TracebackSession [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on code in PR #38885:
URL: https://github.com/apache/airflow/pull/38885#discussion_r1558340800
##########
tests/core/test_settings.py:
##########
@@ -276,27 +286,22 @@ def test_encoding_absent_in_v2(is_v1, mock_conf):
("core", "internal_api_url"): "http://localhost:8888",
}
)
-def test_get_traceback_session_if_aip_44_enabled():
+def test_get_traceback_session_if_aip_44_enabled(clear_internal_api):
# ensure we take the database_access_isolation config
InternalApiConfig._init_values()
assert InternalApiConfig.get_use_internal_api() is True
- # ensure that the Session object is TracebackSession
- configure_orm()
-
- from airflow.settings import Session
-
- assert Session == TracebackSession
Review Comment:
removed this part of the test so we can avoid reconfigure orm
it will me an that the behavior that settings.Session is traceback session when using db isolation will be untested -- but, maybe that's acceptable
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix test side effects from TracebackSession [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on code in PR #38885:
URL: https://github.com/apache/airflow/pull/38885#discussion_r1558335216
##########
tests/core/test_settings.py:
##########
@@ -278,25 +278,25 @@ def test_encoding_absent_in_v2(is_v1, mock_conf):
)
def test_get_traceback_session_if_aip_44_enabled():
# ensure we take the database_access_isolation config
- InternalApiConfig._init_values()
- assert InternalApiConfig.get_use_internal_api() is True
+ try:
+ InternalApiConfig._init_values()
+ assert InternalApiConfig.get_use_internal_api() is True
- # ensure that the Session object is TracebackSession
- configure_orm()
+ with create_session() as session:
+ assert isinstance(session, TracebackSession)
- from airflow.settings import Session
+ # no error just to create the session
+ # but below, when we try to use, it will raise
- assert Session == TracebackSession
Review Comment:
removed this part of the test so we can avoid reconfigure orm
it will me an that the behavior that `settings.Session` is traceback session when using db isolation will be _untested_ -- but, maybe that's acceptable
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix test side effects from TracebackSession [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #38885:
URL: https://github.com/apache/airflow/pull/38885#discussion_r1558339250
##########
tests/core/test_settings.py:
##########
@@ -65,6 +65,16 @@ def task_must_have_owners(task: BaseOperator):
"""
+@pytest.fixture
+def clear_internal_api():
Review Comment:
Yes. better with fixture.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org