Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[improve][io] KCA: flag to force optional primitive schemas #19951

Merged
merged 1 commit into from
Mar 29, 2023

Conversation

dlg99
Copy link
Contributor

@dlg99 dlg99 commented Mar 28, 2023

Motivation

Kafka's schema has "Optional" flag that used there to validate data/allow nulls.
Pulsar's schema does not have such info which makes conversion to kafka schema lossy.

Modifications

Added a config parameter that lets one force primitive schemas into optional ones.
KV schema is always optional.

Default is false, to match existing behavior.

Verifying this change

  • Make sure that the change passes the CI checks.

This change updated tests.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: dlg99#12

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 28, 2023
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

+1

@dlg99 dlg99 force-pushed the kca-schema-optional branch from 71b4444 to c688a22 Compare March 29, 2023 17:12
@codecov-commenter
Copy link

Codecov Report

Merging #19951 (c688a22) into master (7a99e74) will increase coverage by 40.91%.
The diff coverage is 79.37%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #19951       +/-   ##
=============================================
+ Coverage     31.88%   72.79%   +40.91%     
- Complexity     6421    31540    +25119     
=============================================
  Files          1682     1859      +177     
  Lines        127354   137175     +9821     
  Branches      13892    15109     +1217     
=============================================
+ Hits          40601    99857    +59256     
+ Misses        80714    29357    -51357     
- Partials       6039     7961     +1922     
Flag Coverage Δ
inttests 24.41% <1.13%> (-0.11%) ⬇️
systests 25.02% <1.61%> (-0.07%) ⬇️
unittests 72.08% <79.37%> (+54.80%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../loadbalance/extensions/data/BrokerLookupData.java 88.88% <0.00%> (+88.88%) ⬆️
...r/loadbalance/extensions/models/UnloadCounter.java 93.84% <ø> (+93.84%) ⬆️
...adbalance/extensions/scheduler/SplitScheduler.java 75.00% <20.00%> (+75.00%) ⬆️
...ensions/strategy/LeastResourceUsageWithWeight.java 81.66% <25.00%> (+81.66%) ⬆️
...dbalance/extensions/ExtensibleLoadManagerImpl.java 76.07% <36.11%> (+73.37%) ⬆️
...sar/common/policies/data/stats/TopicStatsImpl.java 93.87% <50.00%> (+59.39%) ⬆️
...ache/pulsar/broker/namespace/NamespaceService.java 70.05% <62.50%> (+33.68%) ⬆️
...xtensions/channel/ServiceUnitStateChannelImpl.java 80.92% <73.52%> (+80.39%) ⬆️
...r/delayed/bucket/BucketDelayedDeliveryTracker.java 82.12% <76.74%> (+82.12%) ⬆️
...afka/connect/schema/PulsarSchemaToKafkaSchema.java 79.46% <77.41%> (ø)
... and 17 more

... and 1443 files with indirect coverage changes

@dlg99 dlg99 merged commit 55523ac into apache:master Mar 29, 2023
dlg99 added a commit that referenced this pull request Mar 29, 2023
Motivation
Kafka's schema has "Optional" flag that used there to validate data/allow nulls.
Pulsar's schema does not have such info which makes conversion to kafka schema lossy.

Modifications
Added a config parameter that lets one force primitive schemas into optional ones.
KV schema is always optional.

Default is false, to match existing behavior.

(cherry picked from commit 55523ac)
dlg99 added a commit to datastax/pulsar that referenced this pull request Mar 29, 2023
…9951)

Motivation
Kafka's schema has "Optional" flag that used there to validate data/allow nulls.
Pulsar's schema does not have such info which makes conversion to kafka schema lossy.

Modifications
Added a config parameter that lets one force primitive schemas into optional ones.
KV schema is always optional.

Default is false, to match existing behavior.

(cherry picked from commit 55523ac)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants