Bungie-net / api

Resources for the Bungie.net API
Other
1.22k stars 92 forks source link

Issues in Swagger Generated Code #357

Open nine13tech opened 6 years ago

nine13tech commented 6 years ago

I know you are working on this later because right now the gen is all about the docs, but I am posting this for your work at said later date. This is from the Python language output as run under Python 3.6. Also - "noqa:" I assume that is shorthand for there is no QA on these points?

from swagger_client.models.bungie_membership_type import BungieMembershipType # noqa: F401,E501 results in ImportError: cannot import name 'BungieMembershipType' should be: import swagger_client.models.bungie_membership_type

from swagger_client.models.destiny_historical_stats_definitions_destiny_activity_mode_type import DestinyHistoricalStatsDefinitionsDestinyActivityModeType # noqa: F401,E501 results in ImportError: cannot import name 'DestinyHistoricalStatsDefinitionsDestinyActivityModeType' should be: import swagger_client.models.destiny_historical_stats_definitions_destiny_activity_mode_type

from swagger_client.models.trending_trending_entry import TrendingTrendingEntry # noqa: F401,E501 results in ImportError: cannot import name 'TrendingTrendingEntry' should be import swagger_client.models.trending_trending_entry

I'll post more here as I find them. 9er

nine13tech commented 6 years ago

In the generated api_client.py on line 54 'long': int if six.PY3 else long, # noqa: F821 In Python 3.4+ there is no variable type of long() commenting this to: 'long': int, # noqa: F821 is equivelent from under the heading 'long': http://python3porting.com/differences.html Python 2 has two integer types int and long. These have been unified in Python 3, so there is now only one type, int.

nine13tech commented 6 years ago

In the generated README.md on line 55 it states the instantiation of the swagger client should be: api_instance = swagger_client.CommunityContentApi() but that should be api_instance = swagger_client.CommunityContentApi(api_client=conf.api_client) also, there is no inclusionf or the X-API-Key field which should be done like this:

conf = Configuration()
conf.api_key_prefix = {"<prefix_name>": ""}
conf.api_key = {
    "<prefix_name>": "<your_app_api_key_from_bungie>"
}
conf.api_client = ApiClient(None, "X-API-Key", conf.get_api_key_with_prefix("<prefix_name>"))
should be what ever you desire to call this instance... dev, test, prod etc. This allows you to have your API keys in an .env or settings file by placing this in the settings file and calling it in the classes you create: ```python conf.api_key = { "": "" } ```
vthornheart-bng commented 6 years ago

Hmm - with these, are you sure these are errors in our swagger spec and not the generator you're using? We've definitely got spec problems that will prevent generators from working properly as-is: but these sound more like problems with the generator itself. Is this output from Swagger Codegen?

It's definitely possible that one of our spec issues are the source of these issues - but we'll have to hunt that down and find the true root cause before we can do anything with this information. The output there sounds like it's the generator not including all of the data in output (for the OP and the second reply). In the first reply, is this an error because our specs are outputting longs and the codegen assumes all numerical values must be defined as ints?

I also don't know what the messages mean by "noqa" - it looks like that is a message being output by the codegen tool you are running, but you'll have to hit up whoever made that for more info. That might be a good step to take for now - head to wherever you found the codegen tool and ask them what the error messages you are getting mean. They could be spec failings on our part - for instance, perhaps those types that it's not finding aren't being output by us? (though without looking, I know we're at least outputting BungieMembershipType) But in the first reply, it at least sounds like it's the codegen making an unreasonable expectation: the swagger spec has to be written to support many languages, but it sounds like it's expecting no 64 bit numerical types? And in the second reply, it sounds like it's the codegen that's leaving that data out, we have no control over that outputted configuration code that the tool you're using is generating.

Let me know - I may be misinterpreting.

nine13tech commented 6 years ago

It's the swagger-codegen, latest version and built with maven per the docs. I filed this because it's changed since the first commit. (I skipped the second commit but might be able to test). As for the ints vs longs, I think I need to file that on the swagger side. The Python Foundation decided to combine ints and longs into ints in version 3 since python can auto detect the size of the int and use the correct C library; every int is 64 bit. Python2 has variables that can be cast as a long. The 'noqa' is apparently for the lint tests.

vthornheart-bng commented 6 years ago

Ah, good to know! Hmm, well let's keep an eye out on it then - when we loop back around to swagger spec fixes we can figure out which parts require fixes on our end!

nine13tech commented 6 years ago

keeping with the theme of creating a running list for when Thorny The Awesome can get to it It seems that the generator is still outputting swagger-client as the name and version 1.0.0 for the version number.

vthornheart-bng commented 6 years ago

Hmm, those are almost certainly a problem with the generator - I recall that initially we weren't populating the version # correctly, but that's been fixed for a few months. That's one to bring to the folks who wrote that codegen code.

nine13tech commented 6 years ago

I'll open that one on the swagger-codegen project also.