Add description field to ConnectionResponse model (#52692)

[nrnavaneet]

This PR adds the missing description field to the ConnectionResponse model used in the execution API.

Changes Made:

• Added description to the Pydantic ConnectionResponse model in execution_api/datamodels/connection.py.
• Updated the route logic in execution_api/routes/connections.py to include the description in the API response.

Why:

This change ensures that the API response fully represents the Airflow Connection object, aligning with the UI and CLI experience. It resolves issue #52692.

Let me know if further changes or tests are needed!

[pierrejeambrun]

I'm not sure if the models should be perfectly shared between the execution api and the public api.

Maybe the execution api clients (workers etc.. do not need the description at all (yet))

[nrnavaneet]

Thanks for the input! I added the description field for completeness and consistency across UI, CLI, and API. If the execution API clients don’t need it, I’m happy to restrict it to the public API or update it based on your guidance.
Should I move the field to only the public API model instead?

[bugraoz93]

Thanks @nrnavaneet for your first PR! There is already a description field in line 34. If you want to include Field description, feel free, but since the field is str | None, it should also default to None automatically, so I don't think the default=None part is needed in this context.

airflow/airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections.py

Line 34 in 6c4d884

description: str | None

What do you think?

[pierrejeambrun]

I read to fast and thought this was a change in the execution API only datamodels.

ConnectionResponse already has a description field. What are we trying to achieve?

[pierrejeambrun]

I understand now. You're just modifying the wrong datamodel. What you are looking for is:
execution_api/datamodels/connection.py. Indeed there, the ConnectionResponse does not have description field.

I think the execution API has his whole set of Datamodels that are tailored specifically for the execution_api needs. I don't think there is a wish to have unified datamodels by adding extra field there that are not needed.

cc: @ashb

[ashb]

Yeah, I don't know if description ever showed up to authors in the DAG, so I'm not sure this is actually a required change?

cc @gopidesupavan

[gopidesupavan]

Yeah, I don't know if description ever showed up to authors in the DAG, so I'm not sure this is actually a required change?

cc @gopidesupavan

Yeah it just to align description field with the actual connection model, thats fine if it is not required for the task-sdk return response we can ignore that. It just i observed when i was fixing the schema filed :)

[nrnavaneet]

Thanks everyone for the clarification!

As pointed out, the description field already exists in the public API model, and the execution API datamodels are intentionally lean and tailored for their use case. Since the description field is not required for execution API clients and doesn’t serve a functional purpose there, I’ll go ahead and close this PR.

Really appreciate the review and discussion — this helped me understand the separation between the public and execution API models better!

[pierrejeambrun]

Thanks @nrnavaneet for your effort and comprehension, feel free to tackle any other unassigned issues, there are plenty of them and your help is very welcome!

[nrnavaneet]

Thank You @pierrejeambrun