[GitHub] [beam] TheNeuralBit commented on pull request #16909: Introducing KafkaIO.Read implementation compatibility testing

[GitBox <gi...@apache.org>]

TheNeuralBit commented on pull request #16909:
URL: https://github.com/apache/beam/pull/16909#issuecomment-1057408509


   I tend to agree with Cham that this is adding a lot of complexity. But I do want to acknowledge this is a legitimate, frustrating problem you're trying to solve and I'm sorry you had to spend so much time debugging it.
   
   Please understand that we (the Beam community) will need to maintain this code going forward, so we just want to make sure we're not incurring technical debt. Although I suppose you might argue we've already incurred debt and you're helping patch it... 
   
   One specific concern I have with this approach is that it doesn't provide a guarantee that any future properties only supported by one implementation will be added to the `KafkaIIOReadProperties` enum. We're depending on future authors or reviewers to be disciplined and add them to it (please correct me if I'm wrong). This isn't better from the status quo, where authors should have added specific checks rejecting unsupported properties, and they did not. It would be great if we could find a solution that:
   1) Doesn't rely on reflection
   2) Enforces that newly added config properties will need to be handled somehow by both implementations
   3) (optional) keeps information about supported/unsupported config properties on the implementations instead of splitting it in another file
   
   One idea might be to map the Read instance to a Map of config properties (we could do this by inferring a schema with `AutoValueSchema`, but there may be other options), then we could enforce that every property is consumed.
   


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org