search

FundingCircle / jackdaw

A Clojure library for the Apache Kafka distributed streaming platform.
https://fundingcircle.github.io/jackdaw/
BSD 3-Clause "New" or "Revised" License
369 stars 80 forks source link

standalone deser #239

Closed noisesmith closed 4 years ago

noisesmith commented 4 years ago

Updates some stale comments.

Changes the spec for avro serdes so that the schema document is optional. This allows a reader to continue using a topic as the producer evolves their schema, without updating the schemas documents in lockstep.

Checklist

noisesmith commented 4 years ago

I'm still working on the test coverage and CHANGELOG update

creese commented 4 years ago

We may want to create a separate top-level function for when a deserializer-only serde is wanted. That feels a bit safer to me than relaxing the current spec.

codecov[bot] commented 4 years ago

Codecov Report

Merging #239 into master will decrease coverage by 0.27%. The diff coverage is 90.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #239      +/-   ##
==========================================
- Coverage   80.30%   80.02%   -0.28%     
==========================================
  Files          42       42              
  Lines        2660     2673      +13     
  Branches      153      153              
==========================================
+ Hits         2136     2139       +3     
- Misses        371      381      +10     
  Partials      153      153
Impacted Files Coverage Δ
src/jackdaw/serdes/resolver.clj 90.62% <75.00%> (+0.30%) :arrow_up:
src/jackdaw/specs.clj 78.78% <88.88%> (+5.71%) :arrow_up:
src/jackdaw/serdes/avro.clj 89.07% <100.00%> (+0.12%) :arrow_up:
src/jackdaw/serdes/avro/confluent.clj 100.00% <100.00%> (ø)
src/jackdaw/serdes/edn2.clj 45.45% <0.00%> (-45.46%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8ae7dd6...6df5f40. Read the comment docs.

noisesmith commented 4 years ago

We may want to create a separate top-level function for when a deserializer-only serde is wanted. That feels a bit safer to me than relaxing the current spec.

Thanks Charles, I think the current version of the PR does what you want.

cddr commented 4 years ago

I thought that this PR https://github.com/FundingCircle/jackdaw/pull/157 meant that schema was no longer required but looking back, I don't see a test that proves it to be fair.

noisesmith commented 4 years ago

I thought that this PR #157 meant that schema was no longer required but looking back, I don't see a test that proves it to be fair.

I think this is true - but the spec used by the resolver would still fail, and I think I want that entry point.

creese commented 4 years ago

What happened to jackdaw.serdes.avro.confluent/reader-schema?

noisesmith commented 4 years ago

What happened to jackdaw.serdes.avro.confluent/reader-schema?

With a multi-spec a separate top level definition isn't needed - if the :serde-type keys is present with :read-only as the value, the read-only spec is used. This means that resolver stays simple - it doesn't need to know about any other specs, and the spec itself implements that conditional.