You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/06/23 13:40:29 UTC
[GitHub] [kafka] dajac opened a new pull request #8916: MINOR; Provide static constants with lowest and highest supported versions alongside with the schema
dajac opened a new pull request #8916:
URL: https://github.com/apache/kafka/pull/8916
At the moment, the only way available to get the supported versions of a Message is to look at the number of defined schemas, assuming that the lowest version is 0. This PR adds static constants with the versions alongside with the schemas.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on pull request #8916: MINOR; Provide lowest and highest supported versions alongside with the schema in the auto-generated protocol
Posted by GitBox <gi...@apache.org>.
cmccabe commented on pull request #8916:
URL: https://github.com/apache/kafka/pull/8916#issuecomment-657625138
Yeah, this doesn't need a KIP, as @ijuma said.
@dajac, sorry if this is a silly question, but did you look at the `lowestSupportedVersion` and `highestSupportedVersion` accessor functions that we generate for Message types? Do those do what you need?
If we need something in static context, where we don't have an instance, I'd prefer something like `ApiMessageType.PRODUCE.highestSupportedVersion()`. It looks like we don't generate a `lowestSupportedVersion` and `highestSupportedVersion` there, but we could. I think this would work a bit better with the way we actually tend to use these messages, which is with a type enum in most cases.
I guess if there's a really important use-case then we could add the static variables, though. So I'm curious what you have in mind...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe merged pull request #8916: MINOR; Provide lowest and highest supported versions alongside with the schema in the auto-generated protocol
Posted by GitBox <gi...@apache.org>.
cmccabe merged pull request #8916:
URL: https://github.com/apache/kafka/pull/8916
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dajac commented on pull request #8916: MINOR; Provide static constants with lowest and highest supported versions alongside with the schema
Posted by GitBox <gi...@apache.org>.
dajac commented on pull request #8916:
URL: https://github.com/apache/kafka/pull/8916#issuecomment-648155548
@cmccabe What do you think about adding those?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on pull request #8916: MINOR; Provide lowest and highest supported versions alongside with the schema in the auto-generated protocol
Posted by GitBox <gi...@apache.org>.
cmccabe commented on pull request #8916:
URL: https://github.com/apache/kafka/pull/8916#issuecomment-657627827
I think this is not a duplicate of #7226, since as @dajac said, that one is more about simplying `ApiKeys.java`, whereas @dajac 's use case is for a non-request type.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dajac commented on pull request #8916: MINOR; Provide lowest and highest supported versions alongside with the schema in the auto-generated protocol
Posted by GitBox <gi...@apache.org>.
dajac commented on pull request #8916:
URL: https://github.com/apache/kafka/pull/8916#issuecomment-654262576
@ijuma Indeed, it looks similar except that that PR works only for `REQUEST` or `RESPONSE` types. In my case, I wanted to have a way to get versions for a `DATA` type to avoid having to do this: https://github.com/apache/kafka/pull/8897/files#diff-bad29ccb1aba700e1badeff62f1a86b7R47.
We may be able to generalize the idea or to define a registry for `DATA` types. Thoughts?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on pull request #8916: MINOR; Provide lowest and highest supported versions alongside with the schema in the auto-generated protocol
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #8916:
URL: https://github.com/apache/kafka/pull/8916#issuecomment-650785430
This is internal, so doesn't need a KIP. But I think there's a PR that does this already: https://github.com/apache/kafka/pull/7226/files
Can you check @dajac?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe edited a comment on pull request #8916: MINOR; Provide lowest and highest supported versions alongside with the schema in the auto-generated protocol
Posted by GitBox <gi...@apache.org>.
cmccabe edited a comment on pull request #8916:
URL: https://github.com/apache/kafka/pull/8916#issuecomment-657625138
Yeah, this doesn't need a KIP, as @ijuma said.
Looking at the use-case here, I'm +1 on this change. LGTM.
By the way, #9002 adds the concept of pluggable message type enums. But I think that's overkill for what you're doing here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org