pierrejeambrun commented on code in PR #55673:
URL: https://github.com/apache/airflow/pull/55673#discussion_r2355962206
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/plugins.py:
##########
@@ -44,8 +47,22 @@ def get_plugins(
offset: QueryOffset,
) -> PluginCollectionResponse:
plugins_info = sorted(plugins_manager.get_plugin_info(), key=lambda x:
x["name"])
+ valid_plugins: list[PluginResponse] = []
+ for plugin_dict in plugins_info[offset.value :][: limit.value]:
+ try:
+ # Validate each plugin individually
+ plugin = PluginResponse.model_validate(plugin_dict)
+ valid_plugins.append(plugin)
+ except ValidationError as e:
+ logger.warning(
+ "Skipping invalid plugin due to error",
+ plugin_name=plugin_dict.get("name", "<unknown>"),
+ error=str(e),
+ )
+ continue
Review Comment:
Just realized you can't do that. You need to apply the validation before the
'virtual' pagination.
Otherwise the returned page could be 'partial' which isn't correct.
Filter all `plugins_info` and then apply pagination and `total_entries`.
|
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_plugins.py:
##########
@@ -119,6 +119,23 @@ def test_should_response_403(self,
unauthorized_test_client):
response = unauthorized_test_client.get("/plugins")
assert response.status_code == 403
+ def
test_invalid_external_view_destination_should_log_warning_and_continue(self,
test_client, caplog):
+ pytest.importorskip("flask_appbuilder") # Remove after upgrading to
FAB5
+
+ caplog.set_level("WARNING",
"airflow.api_fastapi.core_api.routes.public.plugins")
+
+ response = test_client.get("/plugins")
+ assert response.status_code == 200
+
+ body = response.json()
+ plugin_names = [plugin["name"] for plugin in body["plugins"]]
+
+ # Ensure our invalid plugin is skipped from the valid list
+ assert "test_plugin_invalid" not in plugin_names
+
+ # Verify warning was logged
+ assert any("Skipping invalid plugin due to error" in rec.message for
rec in caplog.records)
Review Comment:
To verify my comment above you could also check the size of items returned,
with a small page count and the appropriate offset (just before the invalid
items) your page size could be smaller that the expected one (even if this is
not the last page)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
