Revanth14 commented on code in PR #68700:
URL: https://github.com/apache/airflow/pull/68700#discussion_r3439391758


##########
airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml:
##########
@@ -12771,6 +12771,8 @@ components:
         port:
           anyOf:
           - type: integer
+            maximum: 65535.0
+            minimum: 1.0

Review Comment:
   Good catch, to be precise about where the bound lands:
   The min/max is only on the request schemas (`ConnectionBody`, 
`ConnectionTestRequestBody`). `ConnectionResponse` used by `GET` and the list 
endpoint, including nested items in `ConnectionCollectionResponse` is a 
separate, unconstrained component. So a client generated from this spec gets a 
response model with no port bound and should not reject a legacy port in a 
response. The constraint only affects request validation.
   
   Server side is covered by 
`test_get_should_return_existing_connection_with_invalid_port` (`port=0` stored 
in metadata DB - `200` with the port serialized).
   
   As you note, such a port was already "invalid at use", so I think enforcing 
it on create/update/test requests while staying lenient on read is the right 
trade-off. If you prefer not to advertise the bound in request schemas either, 
I can switch to validating solely in the `Connection` model and translate that 
`ValueError` at the route boundary.
   



-- 
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]

Reply via email to