clintropolis commented on issue #7256: fix SequenceMetadata deserialization URL: https://github.com/apache/incubator-druid/pull/7256#issuecomment-472579710 Thanks for review @gianm and @jihoonson, will open a follow up PR with the fix for restoring a task that only needs to publish and is already at it's end offset. >What don't you love about it? 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.
---------------------------------------------------------------- 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]
