Skip to content

STORM-1901: Avro Integration for Storm-Kafka - #1521

Closed
vesense wants to merge 4 commits into
apache:masterfrom
vesense:STORM-1901
Closed

STORM-1901: Avro Integration for Storm-Kafka#1521
vesense wants to merge 4 commits into
apache:masterfrom
vesense:STORM-1901

Conversation

@vesense

@vesensevesense commented Jun 26, 2016

Copy link
Copy Markdown
Member

init version for code review, changes are following:

  • separate Avro from storm-hdfs as a new subproject. (can used by storm-hdfs & storm-kafka, or others modules)
  • add new Config.TOPOLOGY_AVRO_SERIALIZER in Config
  • add new Config.TOPOLOGY_AVRO_CONFLUENT_SCHEMA_REGISTRY_URL in Config
  • add DirectAvroSerializer and DefaultDirectAvroSerializer in storm-avro
  • add new AvroSchema in storm-kafka
  • add new DirectAvroTupleToKafkaMapper in storm-kafka
  • new unit tests
  • document improvements

@vesense

vesense commented Jun 26, 2016

Copy link
Copy Markdown
MemberAuthor

@dossett I add you as the Committer Sponsors because you have done a lot in storm avro integration . Hope you have chance to take a look.

@vesensevesense closed this Jun 26, 2016
@vesensevesense reopened this Jun 26, 2016
@vesensevesense closed this Jun 26, 2016
@vesensevesense reopened this Jun 26, 2016
@vesense

Copy link
Copy Markdown
MemberAuthor

@ptgoetz@HeartSaVioR Could you also take a look?

@dossett

Copy link
Copy Markdown
Contributor

Thanks @vesense I will review this week.

@harshach

Copy link
Copy Markdown
Contributor

@vesense is this intended to be a schema registry, i.e users publishing their schemas IMO having schema registry again in storm is not a good idea.
By looking at the code seems to be a module containing avro related modules and mostly related to storm-kafka module. From what I see this is a AvroSerializer and AvroDeserializer if so shouldn't these part of storm-kafka module instead of being its own external module. Trying to understand where its applicable except the storm-kafka module at this point.

Comment threadexternal/storm-kafka/README.md Outdated

###AvroSchema

`AvroSchema` is an implementation of `Schema`. You can use AvroSchema to read Avro GenericRecord from Kafka:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be AvroScheme and Scheme

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx. Will fix.

@dossett

Copy link
Copy Markdown
Contributor

@harshach My interpretation of the PR is that it moves some Avro speficif code I added to storm-hdfs into a more sharable module.

@vesense I like the idea of making it easier to read/write avro data in a Storm topology, thanks for looking at this. I don't know that it needs to be its own module though, maybe it could be somewhere in org.apache.storm.utils ?

@vesense

Copy link
Copy Markdown
MemberAuthor

The reason why I define the new interface DirectAvroSerializer and move avro to a sharable module is explained as above.

@harshach This PR is based on STORM-1504, you can take a look at it.

@dossett Your reviews have been addressed. Please have a look.

@vesense

Copy link
Copy Markdown
MemberAuthor

@dossett BTW, I have a question about ConfluentAvroSerializer:
I can't see any unit tests for ConfluentAvroSerializer. I wonder how kryo create the object by using construction ConfluentAvroSerializer(Kryo k, Map stormConf) with parameters (ConfluentAvroSerializer is specified by avro.serializer).

d2r pushed a commit to d2r/storm that referenced this pull request Oct 16, 2018
We are closing stale Pull Requests to make the list more manageable.
Please re-open any Pull Request that has been closed in error.
Closesapache#608Closesapache#639Closesapache#640Closesapache#648Closesapache#662Closesapache#668Closesapache#692Closesapache#705Closesapache#724Closesapache#728Closesapache#730Closesapache#753Closesapache#803Closesapache#854Closesapache#922Closesapache#986Closesapache#992Closesapache#1019Closesapache#1040Closesapache#1041Closesapache#1043Closesapache#1046Closesapache#1051Closesapache#1078Closesapache#1146Closesapache#1164Closesapache#1165Closesapache#1178Closesapache#1213Closesapache#1225Closesapache#1258Closesapache#1259Closesapache#1268Closesapache#1272Closesapache#1277Closesapache#1278Closesapache#1288Closesapache#1296Closesapache#1328Closesapache#1342Closesapache#1353Closesapache#1370Closesapache#1376Closesapache#1391Closesapache#1395Closesapache#1399Closesapache#1406Closesapache#1410Closesapache#1422Closesapache#1427Closesapache#1443Closesapache#1462Closesapache#1468Closesapache#1483Closesapache#1506Closesapache#1509Closesapache#1515Closesapache#1520Closesapache#1521Closesapache#1525Closesapache#1527Closesapache#1544Closesapache#1550Closesapache#1566Closesapache#1569Closesapache#1570Closesapache#1575Closesapache#1580Closesapache#1584Closesapache#1591Closesapache#1600Closesapache#1611Closesapache#1613Closesapache#1639Closesapache#1703Closesapache#1711Closesapache#1719Closesapache#1737Closesapache#1760Closesapache#1767Closesapache#1768Closesapache#1785Closesapache#1799Closesapache#1822Closesapache#1824Closesapache#1844Closesapache#1874Closesapache#1918Closesapache#1928Closesapache#1937Closesapache#1942Closesapache#1951Closesapache#1957Closesapache#1963Closesapache#1964Closesapache#1965Closesapache#1967Closesapache#1968Closesapache#1971Closesapache#1985Closesapache#1986Closesapache#1998Closesapache#2031Closesapache#2032Closesapache#2071Closesapache#2076Closesapache#2108Closesapache#2119Closesapache#2128Closesapache#2142Closesapache#2174Closesapache#2206Closesapache#2297Closesapache#2322Closesapache#2332Closesapache#2341Closesapache#2377Closesapache#2414Closesapache#2469
d2r pushed a commit to d2r/storm that referenced this pull request Oct 16, 2018
We are closing stale Pull Requests to make the list more manageable.
Please re-open any Pull Request that has been closed in error.
Closesapache#608Closesapache#639Closesapache#640Closesapache#648Closesapache#662Closesapache#668Closesapache#692Closesapache#705Closesapache#724Closesapache#728Closesapache#730Closesapache#753Closesapache#803Closesapache#854Closesapache#922Closesapache#986Closesapache#992Closesapache#1019Closesapache#1040Closesapache#1041Closesapache#1043Closesapache#1046Closesapache#1051Closesapache#1078Closesapache#1146Closesapache#1164Closesapache#1165Closesapache#1178Closesapache#1213Closesapache#1225Closesapache#1258Closesapache#1259Closesapache#1268Closesapache#1272Closesapache#1277Closesapache#1278Closesapache#1288Closesapache#1296Closesapache#1328Closesapache#1342Closesapache#1353Closesapache#1370Closesapache#1376Closesapache#1391Closesapache#1395Closesapache#1399Closesapache#1406Closesapache#1410Closesapache#1422Closesapache#1427Closesapache#1443Closesapache#1462Closesapache#1468Closesapache#1483Closesapache#1506Closesapache#1509Closesapache#1515Closesapache#1520Closesapache#1521Closesapache#1525Closesapache#1527Closesapache#1544Closesapache#1550Closesapache#1566Closesapache#1569Closesapache#1570Closesapache#1575Closesapache#1580Closesapache#1584Closesapache#1591Closesapache#1600Closesapache#1611Closesapache#1613Closesapache#1639Closesapache#1703Closesapache#1711Closesapache#1719Closesapache#1737Closesapache#1760Closesapache#1767Closesapache#1768Closesapache#1785Closesapache#1799Closesapache#1822Closesapache#1824Closesapache#1844Closesapache#1874Closesapache#1918Closesapache#1928Closesapache#1937Closesapache#1942Closesapache#1951Closesapache#1957Closesapache#1963Closesapache#1964Closesapache#1965Closesapache#1967Closesapache#1968Closesapache#1971Closesapache#1985Closesapache#1986Closesapache#1998Closesapache#2031Closesapache#2032Closesapache#2071Closesapache#2076Closesapache#2108Closesapache#2119Closesapache#2128Closesapache#2142Closesapache#2174Closesapache#2206Closesapache#2297Closesapache#2322Closesapache#2332Closesapache#2341Closesapache#2377Closesapache#2414Closesapache#2469
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@vesense@dossett@harshach