bugraoz93 commented on PR #48914:
URL: https://github.com/apache/airflow/pull/48914#issuecomment-2800112818

   > > I think it's ready for review. Need to fix up the mypy failure still. I 
manually added the ignore assignment. Will look into that tomorrow, but the 
rest should be good I believe.
   > 
   > We should also include automation `auth_generated` for our login models.
   > 
   > 
https://github.com/apache/airflow/blob/0ca0f17996c86efb292cf5b10181944c67e3b862/airflow-ctl/pyproject.toml#L112
   > 
   > 
   > 
https://github.com/apache/airflow/blob/main/airflow-ctl/src/airflowctl/api/datamodels/auth_generated.py
   > > @bugraoz93 I used the 
airflow-core/src/airflow/api_fastapi/core_api/openapi/v1-generated.yaml to not 
have to start the server just to do the codegen? But I guess it's creating a 
bunch more models than it should? I'm not too sure but my assumption was the 
generated yaml file should be the same api exposed by the server and thus we 
should've gotten the same output? Also this is causing some tests to fail which 
also confuses me a bit. 🤔 Do you have any thoughts?
   > 
   > It seems like Python 3.9 doesn't like the annotation, but we have `from 
__future__ ...` there. Checking.
   
   @aritra24  This part `class ExtraLinksResponse(RootModel[dict[str, str | 
None] | None]):` threws.
   
   ``` Python
   TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'
   ```
   
   This occurs because the syntax str | None is not supported in Python 3.9. 
This union operator (|) for type hints was introduced in Python 3.10. Although 
from __future__ import annotations postpones the evaluation of type 
annotations, it doesn't enable the use of the | operator for type unions in 
earlier Python versions.​ Since we are calling the class in tests, it throws 
the error.
   
   We can exclude some models in Custom Formatter but that could cause some 
models to be missing and want to be used in aitflowctl. So we can also find out 
why this model is coming from `RootModel` and not from `from 
airflow.api_fastapi.core_api.base import BaseModel` :thinking: Because if it 
would use BaseModel, we won't get this problem at all.
   
   


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