Skip to content

Conversation

@ceridwen
Copy link
Contributor

I'm pulling the OpenAPI spec from running OpenShift clusters using the /swagger.json endpoint and introspecting information about the Python client. To make sure the names match, I'm running preprocess_spec.py on it before I examine it. When I do, I get a KeyError when the operation doesn't have the 'tags' key, for instance:

operation = {'description': 'create a Template', 'consumes': ['*/*'], 'produces': ['application/json', 'application/yaml', 'application/vnd.kubernetes.protobuf'], 'schemes': ['https'], 'operationId': 'createNamespacedProcessedTemplateV1', 'parameters': [{'name': 'body', 'in': 'body', 'required': True, 'schema': {'$ref': '#/definitions/com.github.openshift.origin.pkg.template.apis.template.v1.Template'}}], 'responses': {'200': {'description': 'OK', 'schema': {'$ref': '#/definitions/com.github.openshift.origin.pkg.template.apis.template.v1.Template'}}, '401': {'description': 'Unauthorized'}}}

This pull request simply ignores operations without the tags key. I don't know why you don't see this when running this script to generate the client or that this is the right fix. If the issue is something else, I can amend my pull request.

@fabianvf
Copy link
Member

fabianvf commented Nov 16, 2017

Good catch, also not sure why I'm not seeing it (what version of openshift/kubernetes are you pulling?).

Unfortunately there's a gross bit of indirection here, the preprocess_spec.py file you modified is actually generated from the file in the kubernetes generator [1], slightly modified to support the delta for the openshift spec. If you want to modify the scripts/from_gen/preprocess_spec.py file, you'll have to look at the scripts/get_scripts_from_gen.sh and scripts/preprocess_spec.py.stub files. Otherwise, your change will be overwritten next time the scripts are pulled from the kubernetes project.

  1. https://github.com/kubernetes-client/gen/blob/master/openapi/preprocess_spec.py

@fabianvf fabianvf self-requested a review November 16, 2017 16:33
@ceridwen
Copy link
Contributor Author

I'm seeing this error running against OpenShift 3.6. My best guess as to why you haven't seen it is that it's nondeterministic and/or you're running on a different Python version than I am. (This issue appears on 3.6.) The other issue I filed yesterday is definitely nondeterministic and appears only on Python 3.5, AFAICT. The nondeteterminism is coming from hash randomization and iteration over dicts: I had to set PYTHONHASHSEED to track the other issue down.

@ceridwen
Copy link
Contributor Author

I sent in a pull request to fix this upstream. I'm going to wait at least a little bit to see if it gets uptake there before I hassle with altering that stub file. If the fix goes in upstream, you should be able to rebuild the files here and push a bugfix release to PyPi?

@fabianvf
Copy link
Member

@ceridwen yeah probably. Might need to tweak some stuff if much has changed but (knock on wood) it should be pretty easy

@fabianvf
Copy link
Member

Good call on pushing changes upstream.

@ceridwen
Copy link
Contributor Author

ceridwen commented Jan 9, 2018

They merged my upstream pull request fixing this and #119. Can you pull in the changes? You can close this pull request and that issue afterwards.

@fabianvf
Copy link
Member

yep, we're working on pulling in the latest changes to client-python now

@openshift-bot
Copy link

@ceridwen: PR needs rebase.

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.

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 11, 2018
@fabianvf
Copy link
Member

We no longer support module generation, the new dynamic client and corresponding k8s module are recommended instead.

@fabianvf fabianvf closed this Jun 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants