NASA-PDS / pds-api-client

Python library and API for accessing the online PDS Search API. This repository however only contains the utilities used to generate, test, document and demo the actual pds.api-client package. The library itself is only released on pypi (https://pypi.org/project/pds.api-client/) but not here on github
https://nasa-pds.github.io/pds-api-client/
Apache License 2.0
1 stars 2 forks source link

Stack trace when calling `BundlesApi.bundle_by_lidvid` #6

Closed nutjob4life closed 3 years ago

nutjob4life commented 3 years ago

The pds.api_client 0.6.0 is now producing a stack trace when calling the pds.api_client.BundlesApi method bundle_by_lidvid when called on pds-gamma. This function worked up to 0.5.0.

To reproduce:

$ python3 -m venv venv
$ cd venv
$ bin/pip install --quiet --upgrade setuptools wheel pip
$ bin/pip install pds.api_client==0.6.0
Collecting pds.api_client==0.6.0
  Using cached pds.api_client-0.6.0-py3-none-any.whl (81 kB)
Collecting six>=1.10
  Using cached six-1.16.0-py2.py3-none-any.whl (11 kB)
Collecting python-dateutil
  Using cached python_dateutil-2.8.1-py2.py3-none-any.whl (227 kB)
Collecting urllib3>=1.15
  Using cached urllib3-1.26.6-py2.py3-none-any.whl (138 kB)
Collecting certifi
  Using cached certifi-2021.5.30-py2.py3-none-any.whl (145 kB)
Installing collected packages: six, urllib3, python-dateutil, certifi, pds.api-client
Successfully installed certifi-2021.5.30 pds.api-client-0.6.0 python-dateutil-2.8.1 six-1.16.0 urllib3-1.26.6
$ bin/python
Python 3.9.5 (default, May  4 2021, 03:36:27) 
[Clang 12.0.0 (clang-1200.0.32.29)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import pds.api_client
>>> config = pds.api_client.Configuration()
>>> config.host = 'https://pds-gamma.jpl.nasa.gov/api/'
>>> apiClient = pds.api_client.ApiClient(config)
>>> bundles = pds.api_client.BundlesApi(apiClient)
>>> bundles.bundle_by_lidvid('urn:nasa:pds:insight_documents::2.0')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/private/tmp/foobar/venv/lib/python3.9/site-packages/pds/api_client/api/bundles_api.py", line 62, in bundle_by_lidvid
    return self.bundle_by_lidvid_with_http_info(lidvid, **kwargs)  # noqa: E501
  File "/private/tmp/foobar/venv/lib/python3.9/site-packages/pds/api_client/api/bundles_api.py", line 136, in bundle_by_lidvid_with_http_info
    return self.api_client.call_api(
  File "/private/tmp/foobar/venv/lib/python3.9/site-packages/pds/api_client/api_client.py", line 365, in call_api
    return self.__call_api(resource_path, method,
  File "/private/tmp/foobar/venv/lib/python3.9/site-packages/pds/api_client/api_client.py", line 209, in __call_api
    return_data = self.deserialize(response_data, response_type)
  File "/private/tmp/foobar/venv/lib/python3.9/site-packages/pds/api_client/api_client.py", line 281, in deserialize
    return self.__deserialize(data, response_type)
  File "/private/tmp/foobar/venv/lib/python3.9/site-packages/pds/api_client/api_client.py", line 320, in __deserialize
    return self.__deserialize_model(data, klass)
  File "/private/tmp/foobar/venv/lib/python3.9/site-packages/pds/api_client/api_client.py", line 659, in __deserialize_model
    kwargs[attr] = self.__deserialize(value, attr_type)
  File "/private/tmp/foobar/venv/lib/python3.9/site-packages/pds/api_client/api_client.py", line 302, in __deserialize
    return {k: self.__deserialize(v, sub_kls)
  File "/private/tmp/foobar/venv/lib/python3.9/site-packages/pds/api_client/api_client.py", line 302, in <dictcomp>
    return {k: self.__deserialize(v, sub_kls)
  File "/private/tmp/foobar/venv/lib/python3.9/site-packages/pds/api_client/api_client.py", line 309, in __deserialize
    klass = getattr(pds.api_client.models, klass)
AttributeError: module 'pds.api_client.models' has no attribute 'list'
al-niessner commented 3 years ago

@jordanpadams

Were be the code? Does not appear to be in this repo nor pds-api-pythonlib. Any hints would be helpful.

nutjob4life commented 3 years ago

I can handle this, @al-niessner! The code is generated from the swagger.json file. See this README. Or you can browse the source distribution and see what gets published and used by dependent code.

nutjob4life commented 3 years ago

Hey! I meant I can handle the question you originally asked @jordanpadams, not handle the issue.

I think a Swagger + Open API Generator expert would be better suited to it. I have little experience with either.

al-niessner commented 3 years ago

lol - I will finish work it then.

I took it to be generated code from the error -- all the getatttr() calls is good hint -- but looking at this set up versus the one in the notebook that I know to work there appears to be no difference. However this is using 0.6.0 and the notebook is using 0.5.0. The older one may be happier than the newer. We will see.

al-niessner commented 3 years ago

Confirmed it works in 0.5.0 but not 0.6.0.

nutjob4life commented 3 years ago

Confirmed it works in 0.5.0 but not 0.6.0.

With a moniker nutjob4life I always question my sanity. Glad to see confirmation!

al-niessner commented 3 years ago

@jordanpadams @nutjob4life

Oh it is getting saner after I made sure to use both versions. There is a single code change that is responsible for the error. I will need help or direction if you want me to actually fix it.

pds.api_client.models.product.py:47

        'properties': 'dict(str, listt)'

to (or back to)

        'properties': 'dict(str, object)'

Which is the only meaningful code change between 0.5.0 and 0.6.0. Going from object to list seems familiar from the registry-app-service code, but is seems to break the auto-generated code. Anyone want to give me direction on how to fix it or is my portion done?

days later....

This also works and may be more inline with the desire design changes:

        'properties': 'dict(str, list[object])'
al-niessner commented 3 years ago

Okay, made it back onto swaggerhub and browsing what I can find. I am looking for the bit that changed between 0.5.0 and 0.6.0. I want to modify it a tad more but cannot find what to change. Any hints?

al-niessner commented 3 years ago

Yeah, simply cannot find 0.6.0 let alone a diff. The latest version I can find on swaggerhub is 0.3.dev.

jordanpadams commented 3 years ago

@tloubrieu-jpl can you poke at this? we may need an additional ticket to track how we can trace swaggerhub API version <-> registry-api-service <-> api-java-lib <-> pds-api-client

tloubrieu-jpl commented 3 years ago

@al-niessner @jordanpadams the pds-api-client versions are upgraded indendantly from the swagger model. 0.6. from the pds-api-client uses the latest stable version for the swagger hub model which is 0.3.0 but apparently is not tagged there. Sorry it is messy. I will take a closer look.

al-niessner commented 3 years ago

@tloubrieu-jpl

What changed between client release 0.5.0 and 0.6.0? Also, where do I look to see what changed?

tloubrieu-jpl commented 3 years ago

hi @al-niessner this repository is a bit weird since it is fully autogenerating the package from the swagger.json file. So the only difference would be in the swagger.json file at the root of the repository.

Apparently 0.6.0 was done with the latest version of the openapi specification currenly in branch 0.3.dev on swaggerhub.

It looks like this bug is related to that https://github.com/OpenAPITools/openapi-generator/issues/5421 since we are using nested dictionnary values. I was albel to reproduce the error with python client_demo.py. I missed that error. I would first try to upgrade the openapi code generator that we use to see if that helps.

After this is solved, we will need to develop some integration testing. I would like to do that with behave which we have tested before with the pds-doi-service repo.

tloubrieu-jpl commented 3 years ago

I was able to make the client work again by doing 2 changes in the generated file api_client.py. See api_client.py.zip

changes are line 66 and 650.

to make that work we need to do a pull request on the openapi code, I think... that should be in the same .mustache file that is cited in thebug above.

tloubrieu-jpl commented 3 years ago

The file we need to change is https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/python-legacy/api_client.mustache

I will try to do a pull request now and see if they take it quickly...

tloubrieu-jpl commented 3 years ago

I created a bug and proposed a pull request, see https://github.com/OpenAPITools/openapi-generator/issues/10005

@jordanpadams their bug and pull request template is well done, we are not far from what they do though. In addition to what we're having they have check lists to verify you have done everything right.

nutjob4life commented 3 years ago

@tloubrieu-jpl yeah, it's pretty nice what they've set up. I love checklists.

tloubrieu-jpl commented 3 years ago

Actually now that I am trying my pull request, from their master branch, I realize that the module I need to use is 'python-legacy'. In the version I am using 4.3.1 on my laptop it was python.

On the latest version, there is another module 'python'. This works with our swagger but the python import required look different (ie not as simple as with python-legacy).

For now I will keep track of what needs to be done to build now, and release a 0.6.1 version but the sustainable way will be to use their new module python. Especially since they might not care to merge a pull request on the 'python-legacy' module.

tloubrieu-jpl commented 3 years ago

I made a pull request for this ticket and produced new package version 0.6.1 available on pypi.

nutjob4life commented 3 years ago

Hi @tloubrieu-jpl I tested with 0.6.1 and it works! 🎉