Skip to content

fix(providers-weaviate): Correct HTTP port logic in WeaviateHook #55457

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.

Sign up for GitHub

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

Closed
ArthurKretzer wants to merge 3 commits into from
Closed

fix(providers-weaviate): Correct HTTP port logic in WeaviateHook #55457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
37ef803
fix: hook port logic and testing
Sep 9, 2025
6cab7c9
Merge branch 'main' into fix/weaviate-hook-port-logic
ArthurKretzer Sep 11, 2025
2f419d0
Merge branch 'main' into fix/weaviate-hook-port-logic
ArthurKretzer Sep 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion providers/weaviate/src/airflow/providers/weaviate/hooks/weaviate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def get_conn(self) -> WeaviateClient:
grpc_secure = extras.pop("grpc_secure", False)
return weaviate.connect_to_custom(
http_host=conn.host, # type: ignore[arg-type]
http_port=conn.port or 443 if http_secure else 80,
http_port=conn.port or (443 if http_secure else 80),
http_secure=http_secure,
grpc_host=extras.pop("grpc_host", conn.host),
grpc_port=extras.pop("grpc_port", 443 if grpc_secure else 80),
Expand Down
10 changes: 5 additions & 5 deletions providers/weaviate/tests/unit/weaviate/hooks/test_weaviate.py
View file
Open in desktop
Copy link
Contributor

@sjyangkevin sjyangkevin Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the fix. wondering if it would be better to have one parameterized test case to specifically test for this port configuration. For example,

if port is not specified and http_secure is False, then 80
if port is not specified and http_secure is True, then 443
if port is set to 8000 and http_secure is False, then 8000
etc.

Copy link
Contributor Author

@ArthurKretzer ArthurKretzer Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. I think that it would, but I must admit that I am not that good with testing frameworks so I have not risked including it at the moment.

Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ def test_get_conn_with_api_key_in_extra(self, mock_connect_to_custom, mock_auth_
mock_auth_api_key.assert_called_once_with(api_key=self.api_key)
mock_connect_to_custom.assert_called_once_with(
http_host=self.host,
http_port=80,
http_port=8000,
http_secure=False,
grpc_host="localhost",
grpc_port=50051,
Expand All @@ -198,7 +198,7 @@ def test_get_conn_with_token_in_extra(self, mock_connect_to_custom, mock_auth_ap
mock_auth_api_key.assert_called_once_with(api_key=self.api_key)
mock_connect_to_custom.assert_called_once_with(
http_host=self.host,
http_port=80,
http_port=8000,
http_secure=False,
grpc_host="localhost",
grpc_port=50051,
Expand All @@ -216,7 +216,7 @@ def test_get_conn_with_access_token_in_extra(self, mock_connect_to_custom, mock_
)
mock_connect_to_custom.assert_called_once_with(
http_host=self.host,
http_port=80,
http_port=8000,
http_secure=False,
grpc_host="localhost",
grpc_port=50051,
Expand All @@ -236,7 +236,7 @@ def test_get_conn_with_client_secret_in_extra(self, mock_connect_to_custom, mock
)
mock_connect_to_custom.assert_called_once_with(
http_host=self.host,
http_port=80,
http_port=8000,
http_secure=False,
grpc_host="localhost",
grpc_port=50051,
Expand All @@ -252,7 +252,7 @@ def test_get_conn_with_client_password_in_extra(self, mock_connect_to_custom, mo
mock_auth_client_password.assert_called_once_with(username="login", password="password", scope=None)
mock_connect_to_custom.assert_called_once_with(
http_host=self.host,
http_port=80,
http_port=8000,
http_secure=False,
grpc_host="localhost",
grpc_port=50051,
Expand Down