gianm commented on issue #7256: fix SequenceMetadata deserialization URL: https://github.com/apache/incubator-druid/pull/7256#issuecomment-472613171 > I don't especially like the abstract method that returns a TypeReference to tell it how to serialize the things, but I guess it's ok compared to the alternatives I can think of like custom json deserializer or creating ParameterizedType with reflection. I do agree that it's nice that pulling out SequenceMetadata does make it a bit more clear the coupling between it and SeekableStreamIndexTaskRunner, I think there is probably some room for refactoring. I can't help but wonder if maybe SeekableStreamIndexTaskRunner should not have been abstract, and rather all of it's abstract methods usage instead be delegating to stream specific types that it encapsulates to do all the things like handle offset comparisons, serde, control points etc, but I haven't quite nailed down what this should look like. Ah. I think the TypeReference based thing you did is pretty reasonable compared to the alternatives. TaskAction does something similar. So that makes me feel better :)
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
