Flume
  1. Flume
  2. FLUME-2227

Move BlobDeserializer from Morphline Sink to flume-ng-core

    Details

    • Type: Bug Bug
    • Status: Patch Available
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: v1.4.0
    • Fix Version/s: None
    • Component/s: Sinks+Sources
    • Labels:
      None

      Description

      This deserializer is more applicable to SpoolDir source and has no dependencies on Morphline sink.

      1. FLUME-2227.1.patch
        26 kB
        Roshan Naik
      2. FLUME-2227.v2.patch
        26 kB
        Roshan Naik
      3. FLUME-2227.v3.patch
        26 kB
        Roshan Naik

        Issue Links

          Activity

          Hide
          Roshan Naik added a comment -

          wolfgang hoschek can you please look into committing this ?

          Show
          Roshan Naik added a comment - wolfgang hoschek can you please look into committing this ?
          Hide
          Santiago M. Mola added a comment -

          This sounds good. Any chance this gets pulled into 1.6.0?

          Show
          Santiago M. Mola added a comment - This sounds good. Any chance this gets pulled into 1.6.0?
          Hide
          Roshan Naik added a comment -

          Removing the protected constr leads to compile error as the base class does not have a default constr.

          updating patch. removing unused imports.

          Show
          Roshan Naik added a comment - Removing the protected constr leads to compile error as the base class does not have a default constr. updating patch. removing unused imports.
          Hide
          Roshan Naik added a comment -

          hi hari .. am a bit a caught up right now... will try to do this later tonight or tomorrow.

          Show
          Roshan Naik added a comment - hi hari .. am a bit a caught up right now... will try to do this later tonight or tomorrow.
          Hide
          Hari Shreedharan added a comment -

          Hey Roshan,

          If you update the patch I will pull this into 1.5

          Show
          Hari Shreedharan added a comment - Hey Roshan, If you update the patch I will pull this into 1.5
          Hide
          Hari Shreedharan added a comment -

          Roshan:

          A couple of things:

          • The old Builder class, by virtue of extending the new builder really builds an instance of the new class and not the old. Since the builder is what is specified in the conf, it is not a problem - maybe we can get rid of even the protected constructor in the BlobDeserializer class?
          • There are a lot of unused imports in the old BlobDeserializer class.

          Once these are fixed I will commit this one.

          Show
          Hari Shreedharan added a comment - Roshan: A couple of things: The old Builder class, by virtue of extending the new builder really builds an instance of the new class and not the old. Since the builder is what is specified in the conf, it is not a problem - maybe we can get rid of even the protected constructor in the BlobDeserializer class? There are a lot of unused imports in the old BlobDeserializer class. Once these are fixed I will commit this one.
          Hide
          Roshan Naik added a comment -

          I renamed it to ResettableTestByteInputStream as the read method in the object actually accepts byte[]

          Show
          Roshan Naik added a comment - I renamed it to ResettableTestByteInputStream as the read method in the object actually accepts byte[]
          Hide
          Hari Shreedharan added a comment -

          Why exactly has ResettableTestStringInputStream's usage replace with a new class in the BlobSerializer tests?

          Show
          Hari Shreedharan added a comment - Why exactly has ResettableTestStringInputStream's usage replace with a new class in the BlobSerializer tests?
          Hide
          Hari Shreedharan added a comment -

          Could you please upload this to rb?

          Show
          Hari Shreedharan added a comment - Could you please upload this to rb?
          Hide
          Roshan Naik added a comment -

          Moving the BlobDeserializer along with tests into core/serialization/
          Also retaining a dummy BlobDeserialzier in morphline sink dir that forwards calls to core/serialization/BlobDeserializer

          Show
          Roshan Naik added a comment - Moving the BlobDeserializer along with tests into core/serialization/ Also retaining a dummy BlobDeserialzier in morphline sink dir that forwards calls to core/serialization/BlobDeserializer
          Hide
          wolfgang hoschek added a comment -

          Good idea as long as existing configs using the old FQCN don't break. New code and doc can use the new FQCN.

          Show
          wolfgang hoschek added a comment - Good idea as long as existing configs using the old FQCN don't break. New code and doc can use the new FQCN.
          Hide
          Hari Shreedharan added a comment -

          Yep, not an issue to doc it.

          Show
          Hari Shreedharan added a comment - Yep, not an issue to doc it.
          Hide
          Roshan Naik added a comment -

          You mean to support the old FQCN ? That would indeed be good idea.
          I am thinking Docs should nevertheless reflect the new FQCN ? That way old configs will continue to work, but newer configs will use the new FQCN.

          Show
          Roshan Naik added a comment - You mean to support the old FQCN ? That would indeed be good idea. I am thinking Docs should nevertheless reflect the new FQCN ? That way old configs will continue to work, but newer configs will use the new FQCN.
          Hide
          Hari Shreedharan added a comment -

          Roshan,

          Since we released this in the last release - let's move it over and make sure that we create a "dummy" inherited class of the same name in the current package itself.

          Show
          Hari Shreedharan added a comment - Roshan, Since we released this in the last release - let's move it over and make sure that we create a "dummy" inherited class of the same name in the current package itself.
          Hide
          Roshan Naik added a comment -

          The right location for deserializers would be.

          flume-ng-core/src/main/java/org/apache/flume/serialization/

          Show
          Roshan Naik added a comment - The right location for deserializers would be. flume-ng-core/src/main/java/org/apache/flume/serialization/

            People

            • Assignee:
              Roshan Naik
              Reporter:
              Roshan Naik
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:

                Development