Use KIP-35/Version API to fetch protocol versions

[tvoinarovskyi]

Currently I see some places like this in code:

        if self.config['api_version'] >= (0, 10):
            version = 2
        elif self.config['api_version'] == (0, 9):
            version = 1
        else:
            version = 0

As of 0.10 this is not quite right, as we have a proper API for Version checking and should use that. I do understand, that Java driver still did not deliver it (KAFKA-3600), but setting up a checker is not that hard to do, and it's still better, than api_version checking.

For example if instead of the check above we could write:

fetch_version = self._client.best_supported_version(FetchRequest)

Any thoughts?

The main issue is probably that we have to do the Version check's asynchronously, but I think I will manage to get over it. I can do a PR if we agree on the need and a bit on API.

[dpkp]

Thanks -- kafka-python checks api version on startup and sets api_version accordingly. It is a single, cluster-wide api version. I dont think we need more complexity (as I discussed during the KIP-35 process). Why do you think a per-method map is needed?

[tvoinarovskyi]

I do see a point there, I agree, that proper implementation of the idea of KIP-35 (at least I understand that it's more for different broker version support of sort) is complicated, but it's not even ready for that. The point is different.

Kafka will keep adding new features, requests and versions. It's evolving every month. With every new update you will add new API version checks on startup. For example in #857 you will probably add check for version 0.10.1 to check if those methods are supported or not. It will mess up the code further.

What I propose is not something that complex, we just store the results of Protocol Versions on startup, cluster-wide, same as we did with api_version. We even already issue the same request, just never look at the results for it. And just move the code, that selects versions for request into 1 place.

[tvoinarovskyi]

I just kinda want the code for version selection in 1 place. Now that I think about it, it's probably OK to have it based on api_version for now, just move it in 1 place.

[dpkp]

#1169 added a private map of supported versions, so I'm going to close this issue.