[GitHub] [kafka] cmccabe commented on pull request #8703: MINOR: add a getOrCreate function to KRPC collections
cmccabe commented on pull request #8703: URL: https://github.com/apache/kafka/pull/8703#issuecomment-633304837 > Why is it the case that non generated code doesn't know about keys? It's a `Collection` rather than a `Map`. Collections don't have a key type and value type-- they just have an element type. So if you want to remove something from a `Set`, for example, you have to supply a Set element (even if some of the fields in the set element you supply will be ignored for the purpose of lookup, which is likely.) This is more efficient in terms of memory used, but a little less intuitive for Java programmers in some cases. I kind of regret making this tradeoff now, since I think many Java programmers are not familiar with interacting with `Set`, and just want a Map`. If I get some time I could redo it, but it would take some time to do. > It seems pretty awkward to implement methods like this that are not related to the the protocol It is related to the protocol, in that the protocol definition determines which fields are part of the key and which are part of the value. As I said, I could redo this to create separate classes for the Key and Value, which would be slightly more elegant and slightly less efficient. 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: us...@infra.apache.org
[GitHub] [kafka] cmccabe commented on pull request #8703: MINOR: add a getOrCreate function to KRPC collections
cmccabe commented on pull request #8703: URL: https://github.com/apache/kafka/pull/8703#issuecomment-632428809 bq. What is the purpose of this new method? Should we add getOrCreate to ImplicitLinkedHashMultiCollection as well? The purpose is to make it easy to access map elements by their keys. We can't add it to `ImplicitLinkedHashMultiCollection` because only the generated code has the notion of keys. In `ImplicitLinkedHashMultiCollection`, we just have elements which is more awkward to deal with. 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: us...@infra.apache.org
[GitHub] [kafka] cmccabe commented on pull request #8703: MINOR: add a getOrCreate function to KRPC collections
cmccabe commented on pull request #8703: URL: https://github.com/apache/kafka/pull/8703#issuecomment-632428282 Looks like Jenkins is flaking out a bit. ``` FATAL: Unable to delete script file /tmp/jenkins3984496192919521109.sh java.io.EOFException ``` Will re-push to get a clean test run. 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: us...@infra.apache.org
