Added generic Python asyncio generator script. #68
Conversation
|
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
I'm not sure if it's place for "generic" clients. This special repo |
And I hope |
|
Can we just modify the existing script rather than introduce a new one? I don't mind having scripts that are used for non |
|
I did not modify Instead, I copied it and:
This is as generic as I can think of making it right now. It also produces a working client. All my I can modify the PR to change |
|
I don't want lots of slightly different scripts. I think you can add conditionalization to make this work correctly with a single |
|
Done. |
|
Sorry, this isn't quite what I hoped, the if/else should minimize the differences between the files, rather than just have a giant if/else across the two blocks. There are many environment variables (e.g. CLEANUP_DIRS) which are shared in the two blocks and shouldn't be duplicated. Please refactor to minimize differences. Thanks (and apologies for not being clear before) |
|
Is this what you had in mind? |
|
Yep, that looks much better, thanks! @tomplus can you validate that Thanks! |
openapi/python-asyncio.sh
Outdated
| CLIENT_LANGUAGE=python-asyncio; \ | ||
| CLEANUP_DIRS=(client/apis client/models docs test); \ | ||
| # Client specific Swagger branch to use. | ||
| if [ ${PACKAGE_NAME} == "kubernetes_asyncio" ]; then |
There was a problem hiding this comment.
PACKAGE_NAME is "client" in my case and it's the same as in the standard python client. Please take a look: https://github.com/kubernetes-client/python/blob/master/scripts/update-client.sh#L49
openapi/python-asyncio.sh
Outdated
| find "${OUTPUT_DIR}/client/" -type f -name \*.py -exec sed -i 's/import client\./import kubernetes_asyncio.client./g' {} + | ||
| find "${OUTPUT_DIR}/client/" -type f -name \*.py -exec sed -i 's/from client/from kubernetes_asyncio.client/g' {} + | ||
| find "${OUTPUT_DIR}/client/" -type f -name \*.py -exec sed -i 's/getattr(client\.models/getattr(kubernetes_asyncio.client.models/g' {} + | ||
| find "${OUTPUT_DIR}/test" -type f -name \*.py -exec sed -i "s/\\bclient/${PACKAGE_NAME}.client/g" {} + |
There was a problem hiding this comment.
There is the same situation with PACKAGE_NAME here and below too.
There was a problem hiding this comment.
Not sure what you want me to change here because this line is a verbatim copy of the one in master.
There was a problem hiding this comment.
The current version does s/\bclient/kubernetes_asyncio.client/g but this wants to do s/\\bclient/client.client/g (because PACKAGE_NAME=client).
|
@tomplus can we get a glance here to see if this does the right things? Thanks! |
|
@brendandburns Thanks for asking. I tested it and it works for me. /lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brendandburns, olitheolix The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@olitheolix this is finally merging :) thanks for your patience. |
This is similar to the existing
python-asyncio.shscript but does not hard code the name of the client library (kubernetes_asyncio). Instead, it uses thePACKAGE_NAMEvariable from the settings file as explained in the Readme to this repository.This will allow others to create their own client libraries more easily.