You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/07/22 19:31:43 UTC
[GitHub] [airflow] WingCode opened a new pull request #17174: Fix Basic Auth Failure OpenApi codegen
WingCode opened a new pull request #17174:
URL: https://github.com/apache/airflow/pull/17174
This fixes basic auth not getting picked up in the openApi generated client code. The example specified here https://swagger.io/docs/specification/authentication/ also uses the same configuration to achieve basic auth in codegen clients
closes: apache#17172
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-885177904
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) for testing locally, itโs a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better ๐.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] WingCode commented on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
WingCode commented on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-885378294
@mik-laj Do you think we can probably maintain the airflow-java-client with the language specific patch you in this repo https://github.com/apache/airflow-client-java?
Applying OpenApi codegen on the current OpenApi specification YAML doesn't work for Java Client as mentioned over here #17172
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] WingCode closed pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
WingCode closed pull request #17174:
URL: https://github.com/apache/airflow/pull/17174
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-949369026
If we need, we can modify the `security` sections during client generation to add support for authentication methods supported by a specific API client generator. The problem is that the specification is not specific about how the client API generator should behave when not all options are supported, or no options are specified. Generator Client API for Golang in this case adds support for all authentication methods. On the other hand, the Python client does not add any method support. It also doesn't describe how the API client generator should support [custom authentication methods](http://airflow.apache.org/docs/apache-airflow/stable/security/api.html#roll-your-own-api-authentication).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-885269516
I am not sure if this is a good solution because we support many authentication methods. I have the impression that OpenAPI clients should support all authentication methods, if we do not specify a specific authentication method in the specification. It already works for Python and Go.
If you have a problem generating the client for a specific language because of a different implementation, you can apply patches while generating the client. For example, see: https://github.com/kubernetes-client/gen/blob/master/openapi/preprocess_spec.py
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-885379912
Yes. airflow-client-* repos are automatically generated. For scripts, see: https://github.com/apache/airflow/tree/main/clients
If you want to develop a client then you should start from this directory. Once successful you need to find an PMC member to validate the code and start the release process.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pateash commented on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
pateash commented on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-890582941
@mik-laj I am also trying to resolve the same issue. In Docs https://swagger.io/docs/specification/authentication/
It clearly mentioned
> After you have defined the security schemes in the securitySchemes section, you can apply them to the whole API or individual operations by adding the security section on the root level or operation level,
@msumit @kaxil I am not sure, how this is working In Python.
It's working for me in Java, after defining schemaSchemas in spec file.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-885269516
I am not sure if this is a good solution because we support many authentication methods. I am not sure if this is a good solution because we support many authentication methods. I have the impression that OpenAPI clients should support all authentication methods, if we do not specify a specific authentication method in the specification. It already works for Python and Go.
If you have a problem generating the client for a specific language because of a different implementation, you can apply patches while generating the client. For example, see: https://github.com/kubernetes-client/gen/blob/master/openapi/preprocess_spec.py
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-885277541
GkeStartPodOperator is mentioned also in the documentation for the generic Kubernetes operator.
https://github.com/apache/airflow/blob/3939e841616d70ea2d930f55e6a5f73a2a99be07/airflow/providers/cncf/kubernetes/operators/kubernetes_pod.py#L64-L68
https://github.com/apache/airflow/blob/8505d2f0a4524313e3eff7a4f16b9a9439c7a79f/docs/apache-airflow-providers-cncf-kubernetes/operators.rst
Do you think it would be worth mentioning your operator there as well?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-885379912
Partially. airflow-client-* repos are automatically generated. For scripts, see: https://github.com/apache/airflow/tree/main/clients
If you want to develop a client then you should start from this directory. When you finish, you need to find an PMC member to validate the code and start the release process.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-885379912
Yes. airflow-client-* repos are automatically generated. For scripts, see: https://github.com/apache/airflow/tree/main/clients
If you want to develop a client then you should start from this directory. When you finish, you need to find an PMC member to validate the code and start the release process.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] hterik commented on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
hterik commented on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-949373573
Is there any drawback of listing all the schemes under security? (Apart from having to keep that list in sync with all securitySchemas defined)
If that works with both go and python client, that approach seems a lot easier than client specific pre-processing of the schema during generation.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-949369026
If we need, we can modify the `security` sections during client generation to add support for authentication methods supported by a specific API client generator. The problem is that the specification is not specific about how the client API generator should behave when not all options are supported, or no options are specified. Generator Client API for Golang in this case adds support for all authentication methods. On the other hand, the Python client does not add any method support.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-885373761
My fsul. I wanted publish this comment in other PR, but i had technically issue with my Macbook and Chrome. Please ignore it.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-949419378
> Is there any drawback of listing all the schemes under security? (Apart from having to keep that list in sync with all securitySchemas defined)
Yes. The API specification and API documentation will display incomplete information which will be confusing. Now we have a custom section that explains authentication. http://airflow.apache.org/docs/apache-airflow/stable/stable-rest-api-ref.html#section/Authentication
If we add to the specification, a new section will be added which will be incomplete/invalid. Here is example:
https://redocly.github.io/redoc/?url=https://petstore.swagger.io/v2/swagger.json
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] hterik commented on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
hterik commented on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-949351457
The way i interpret the spec is also that you have to list all the security schemes under security.
https://swagger.io/docs/specification/authentication/
> After you have defined the security schemes in the securitySchemes section, **you can apply them to the whole API or individual operations by adding the security section on the root level** or operation level, respectively. **When used on the root level, security applies the specified security schemes globally** to all API operations, unless overridden on the operation level
Not listing any scheme does not mean "all".
The only granularity that implicitly means all is the operations, not the security schemes.
I tried making same change as @pateash above on the Python client https://github.com/apache/airflow-client-python, which is broken right now due to not providing the Authentication header, that made it work.
So while this change only listing Basic is not enough. Listing all of Basic+GoogleOpenId+Kerberos should be the way to go.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj removed a comment on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
mik-laj removed a comment on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-885277541
GkeStartPodOperator is mentioned also in the documentation for the generic Kubernetes operator.
https://github.com/apache/airflow/blob/3939e841616d70ea2d930f55e6a5f73a2a99be07/airflow/providers/cncf/kubernetes/operators/kubernetes_pod.py#L64-L68
https://github.com/apache/airflow/blob/8505d2f0a4524313e3eff7a4f16b9a9439c7a79f/docs/apache-airflow-providers-cncf-kubernetes/operators.rst
Do you think it would be worth mentioning your operator there as well?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] WingCode commented on pull request #17174: Fix Basic Auth Failure OpenApi codegen
Posted by GitBox <gi...@apache.org>.
WingCode commented on pull request #17174:
URL: https://github.com/apache/airflow/pull/17174#issuecomment-885372405
> GkeStartPodOperator is mentioned also in the documentation for the generic Kubernetes operator.
> https://github.com/apache/airflow/blob/3939e841616d70ea2d930f55e6a5f73a2a99be07/airflow/providers/cncf/kubernetes/operators/kubernetes_pod.py#L64-L68
>
>
> https://github.com/apache/airflow/blob/8505d2f0a4524313e3eff7a4f16b9a9439c7a79f/docs/apache-airflow-providers-cncf-kubernetes/operators.rst
> Do you think it would be worth mentioning your operator there as well?
I didn't quite get this comment @mik-laj .
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org