You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/09/21 23:18:57 UTC
[GitHub] [airflow] dstandish opened a new pull request, #26579: Fix bad test for unknown hook type
dstandish opened a new pull request, #26579:
URL: https://github.com/apache/airflow/pull/26579
should hopefully fix broken main
--
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
[GitHub] [airflow] Taragolis commented on pull request #26579: Fix bad test for unknown hook type
Posted by GitBox <gi...@apache.org>.
Taragolis commented on PR #26579:
URL: https://github.com/apache/airflow/pull/26579#issuecomment-1254722659
@potiuk Oh, very appreciate for detail explanation
--
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
[GitHub] [airflow] Taragolis commented on pull request #26579: Fix bad test for unknown hook type
Posted by GitBox <gi...@apache.org>.
Taragolis commented on PR #26579:
URL: https://github.com/apache/airflow/pull/26579#issuecomment-1254330914
You a bit faster create PR 😺
It is a bit strange that it wouldn't failed earlier in https://github.com/apache/airflow/pull/25980
--
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
[GitHub] [airflow] Taragolis commented on a diff in pull request #26579: Fix bad test for unknown hook type
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #26579:
URL: https://github.com/apache/airflow/pull/26579#discussion_r977063422
##########
tests/providers/common/sql/operators/test_sql.py:
##########
@@ -226,7 +226,7 @@ def test_get_hook(self, mock_get_conn, database):
def test_not_allowed_conn_type(self, mock_get_conn):
mock_get_conn.return_value = Connection(conn_id='sql_default', conn_type='s3')
- with pytest.raises(AirflowException, match=r"You are trying to use `common-sql`"):
+ with pytest.raises(AirflowException, match=r'Unknown hook type "s3"'):
Review Comment:
I thought better replace conn_type from `s3`, which not exists anymore to existed conn_type like `aws`, `google_cloud_platform`, `http` so we won't change actual test - provide use unsupported Hook
--
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
[GitHub] [airflow] potiuk commented on pull request #26579: Fix bad test for unknown hook type
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #26579:
URL: https://github.com/apache/airflow/pull/26579#issuecomment-1254625487
> It is a bit strange that it wouldn't failed earlier in #25980
It has not failed because we have selective checks that only run porvider's tests for those "changed providers" and those that "depend" on them.
Technically speaking we would run automatically "amazon" tests if "|common.sql" changed (because amazon depends on common.sql):
https://github.com/apache/airflow/blob/main/generated/provider_dependencies.json#L35
But we would not run "common.sql" tests when "amazon" changes because "common.sql" does not depend on amazon:
https://github.com/apache/airflow/blob/main/generated/provider_dependencies.json#L199
The problem in this case was that we had implicit dependency of common.sql "tests" on "amazon" provider - because in the common.sql we were using s3 as example connection, so there was a hidden dependency of "common.sql" tests on "amazon" :).
So actually failing main was our "canary" tests failing showing that there was a test that slipped through the safety net. This is the price to pay for the optimisations we have (we run a small set of provider test now when only providers change - that saves a lot of time and build hours).
The change now changed it to "fs", so we are good as "fs" is not a separate provider but it is part of Airlfow so the impiicit dependency is gone.
--
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
[GitHub] [airflow] jedcunningham merged pull request #26579: Fix bad test for unknown hook type
Posted by GitBox <gi...@apache.org>.
jedcunningham merged PR #26579:
URL: https://github.com/apache/airflow/pull/26579
--
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
[GitHub] [airflow] dstandish commented on a diff in pull request #26579: Fix bad test for unknown hook type
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #26579:
URL: https://github.com/apache/airflow/pull/26579#discussion_r977065853
##########
tests/providers/common/sql/operators/test_sql.py:
##########
@@ -226,7 +226,7 @@ def test_get_hook(self, mock_get_conn, database):
def test_not_allowed_conn_type(self, mock_get_conn):
mock_get_conn.return_value = Connection(conn_id='sql_default', conn_type='s3')
- with pytest.raises(AirflowException, match=r"You are trying to use `common-sql`"):
+ with pytest.raises(AirflowException, match=r'Unknown hook type "s3"'):
Review Comment:
ohhhh... goooood point lemme see
--
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
[GitHub] [airflow] dstandish commented on pull request #26579: Fix bad test for unknown hook type
Posted by GitBox <gi...@apache.org>.
dstandish commented on PR #26579:
URL: https://github.com/apache/airflow/pull/26579#issuecomment-1254332070
haha 😝
--
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
[GitHub] [airflow] dstandish commented on pull request #26579: Fix bad test for unknown hook type
Posted by GitBox <gi...@apache.org>.
dstandish commented on PR #26579:
URL: https://github.com/apache/airflow/pull/26579#issuecomment-1254329612
@Taragolis
--
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