Apikey

[michaz]

Remove API key as an individual query parameter from all paths, and add it as a security specification.

This is clearly the way it is intended, as now I can suddenly use the Swagger tool without entering the API key twice.

[michaz]

.. but I guess half the code generators don't support that properly, right?

[boldtrn]

but I guess half the code generators don't support that properly, right?

I cannot remember why I didn't specified it that way :). It could be that the code generators didn't (or still don't) support this, maybe I just overlooked it? Did you try it out?

[michaz]

It could be that the code generators didn't (or still don't) support this, maybe I just overlooked it? Did you try it out?

Yeah. Looks bad. :(

The code generators seem to be in much worse shape than the specification language itself. At one point we may want to fork a version of the spec which doesn't care about broken code generators.

[boldtrn]

The code generators seem to be in much worse shape than the specification language itself.

Yes and I think a big issue is that some languages simply don't support the concepts that the spec offers. Also I am not familiar with most languages used in the generator, so it's not easy for me (and I think most devs) to actually fix stuff. Sure I can do it in 1 or 2 languages, but we provide hand crafted clients for these languages... ;).

[michaz]

Yes and I think a big issue is that some languages simply don't support the concepts that the spec offers.

.. which would be okay if they were then presenting a "best effort" at presenting the spec in language-native concepts, and if that's not possible, just return (say) a map, or the closest possible thing from which you can then proceed manually.

But for example the API key thing is outright broken in the python client, you just can't set it.

[boldtrn]

But for example the API key thing is outright broken in the python client, you just can't set it.

Oh ok, that sound not ideal. I just checked the codegen repo, there might be related issues here and here. Did you try that or is it not possible at all?

[michaz]

I confirmed issue 7269, but the proposed workaround didn't work for me, and the reason in 7847 (sadly) wasn't the reason for me.

To be clear, that is for my solution (according to the spec, the correct one). Yours works fine. :)

[michaz]

Yours works fine.

And I like that you clarified that that was unintentional. :-D

[michaz]

HAHAHAHAHA, found the root cause. Can we please not name one of our schemas Configuration? It breaks the Python client. :-)

[michaz]

I'm going to sleep now, but I now endorse this pull request again, since I have no further reason to believe it breaks other clients except the Python client, which it breaks for unrelated reasons which are easy to fix.

[karussell]

Glad you figured it out :)

Regarding the configuration thing it might be similar to what we had with break: https://github.com/swagger-api/swagger-codegen/issues/3846 but we do not really know the reserved words

[michaz]

Hmm yeah, but that's something I can live with / understand why this is super-hard to maintain upstream.. I'll try and fix a PR for the documentation issue (#7269) tomorrow.

[michaz]

I'm happy with this now: It works, and if swagger-client accepts my PR, the documentation for Python will also be allright. (And the solution that can even now be found on Google works now, because I renamend Configuration.) Merge?

[karussell]

We need to update the java-examples.

In Java this looks a bit ugly due to the required cast and arbitrary api_key string which is not the URL parameter:

((ApiKeyAuth) geocoding.getApiClient().getAuthentication("api_key")).setApiKey(key);
GeocodingResponse geocodingResponse = geocoding.geocodeGet("bautzen", "de", 5, false, "", "default");

vs. before:

GeocodingResponse geocodingResponse = geocoding.geocodeGet(key, "bautzen", "de", 5, false, "", "default");

[boldtrn]

I think @karussell is right, it might be better to convert api_key to just key? That's the way we name the url parameter.

We need to update the java-examples

Where are these examples?

[karussell]

it might be better to convert api_key to just key?

It is just a notation from swagger. The api key name is already specified under securityDefinitions - at least IMHO.

We need to update the java-examples

Where are these examples?

https://github.com/graphhopper/directions-api-clients/tree/master/java-examples

[boldtrn]

It is just a notation from swagger. The api key name is already specified under securityDefinitions - at least IMHO.

It's a name that we specified in the swagger, so I think we could change it to just key. Which would probably make more sense, since we use just key in the url. But maybe key is a reserved keyword in some programming language and api_key would be the safer option then?

Thanks for pointing me to the examples, I must have missed them :).

[karussell]

@michaz can you write here how this change makes the python code easier / better?

The java code definitely looks worse:

((ApiKeyAuth) geocoding.getApiClient().getAuthentication("api_key")).setApiKey(key);

But maybe this is language specific ...

[michaz]

how this change makes the python code easier / better?

You pass it once to the client, upon construction, instead of per request. That's all, really.

But that's not how I was reasoning. I see it primarily as a specification language, not as an input to a code generator, and I was trying to use it as prescribed. The auto-generated API test / documentation page definitely makes more sense then.

If you're writing the spec "for the code generator", then don't mind me, we're doing different things then.

[karussell]

The auto-generated API test / documentation page definitely makes more sense then.

ok

If you're writing the spec "for the code generator"

The main intend is to get an easier access to our API. So we do this here too, yes. But probably the Java code generator needs an update if python is not much harder. The Java creation code looks really ugly.

[karussell]

And now that Swagger forked into OpenAPITools due to some maintainer issues: https://github.com/OpenAPITools/ I'm not even sure where to post new issues :(

[karussell]

This will change with the new spec and it is required to change the API key via:

ApiClient client = new ApiClient();
client.setApiKey("some key");
api.setApiClient(client);

This will become clear in the java-examples of this pull request: https://github.com/graphhopper/directions-api-clients/pull/38