debayanCODES-1 commented on issue #68382: URL: https://github.com/apache/airflow/issues/68382#issuecomment-5464373057
Hi, I'd like to work on this one. Looking at it, the fix needs to land in six places since port is accepted from six different entry points - core model, task SDK, public REST API, execution API, CLI, and the JSON schema used for execution-time communication. Same validation (0-65535, integer only) in all six spots, but I want to avoid copy-pasting the same range check six times if there's already a shared utility for this kind of thing, or a common base class between a couple of these schemas. A few things I'll figure out along the way, calling them out now in case there's already a preferred answer: - should None/empty port stay allowed since not every connection type needs one, with validation only kicking in when a value is actually given - whether the REST API and execution API schemas share a base class for port already, so one fix covers both - what error format each layer expects (raised exception vs Pydantic validation vs API 422 body) so it matches what's already there Plan is to add tests at each layer for valid boundaries (0, 65535) and invalid ones (-1, 65536, non-integer), plus a check that this fails at creation time and not later inside a hook. Will start digging into the existing code and open a draft PR once I have something working. -- 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]
