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

Aruha 1173 #838

Merged
merged 10 commits into from Apr 5, 2018

Conversation

4 participants
@antban
Member

antban commented Feb 12, 2018

Do not reformat incoming messages while publishing

Zalando ticket : ARUHA-1173

Description

Right now nakadi is parsing json while validating events and than stores events from parsed format to a kafka format. This may lead to problems user behavior (for example 0.0 and 0 should be treated in the same manner, but it is actually not)
This ticket prevents nakadi from this kind of modifications, removing only meaningless values from the events.

@codecov-io

This comment has been minimized.

codecov-io commented Feb 12, 2018

Codecov Report

Merging #838 into master will increase coverage by 0.46%.
The diff coverage is 86.84%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #838      +/-   ##
============================================
+ Coverage     53.18%   53.64%   +0.46%     
- Complexity     1639     1682      +43     
============================================
  Files           310      310              
  Lines          9206     9326     +120     
  Branches        806      837      +31     
============================================
+ Hits           4896     5003     +107     
- Misses         4021     4025       +4     
- Partials        289      298       +9
Impacted Files Coverage Δ Complexity Δ
...ava/org/zalando/nakadi/service/EventPublisher.java 91.48% <ø> (ø) 20 <0> (ø) ⬇️
.../nakadi/repository/kafka/KafkaTopicRepository.java 54.77% <100%> (ø) 52 <0> (ø) ⬇️
.../nakadi/enrichment/MetadataEnrichmentStrategy.java 100% <100%> (ø) 8 <1> (ø) ⬇️
...o/nakadi/controller/EventPublishingController.java 85.5% <100%> (ø) 16 <0> (ø) ⬇️
...n/java/org/zalando/nakadi/domain/BatchFactory.java 85.39% <85.18%> (+10.39%) 52 <47> (+29) ⬆️
...main/java/org/zalando/nakadi/domain/BatchItem.java 85.14% <89.28%> (-10.51%) 24 <3> (+13)
...n/java/org/zalando/nakadi/service/EventStream.java 75% <0%> (+1.61%) 30% <0%> (+1%) ⬆️

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 34bb11f...b85c1dd. Read the comment docs.

batch.add(new BatchItem(sb.toString()));
public static List<BatchItem> from(final String events) {
final List<BatchItem> batch = new ArrayList<>();
int objectStart = locateOpenSquareBracket(events) + 1;

This comment has been minimized.

@adyach

adyach Feb 16, 2018

Member

Maybe arrayStart to be compliant with arrayEnd?
Also why +1 here?

This comment has been minimized.

@antban

antban Feb 16, 2018

Member

@adyach not to parse too much, also, the name is perfectly correct. object start may be only openSquareBracket + 1 and further.

import java.util.Optional;
public class BatchItem {
public enum Injection {
METADATA("metadata"),;

This comment has been minimized.

@adyach
}
return idx;
}
}

This comment has been minimized.

@adyach
event.put("metadata", sourceMetadata);
event.put("foo", "Test data data data");
final BatchItem bi = BatchFactory.from("[" + event.toString(2) + "]").get(0);

This comment has been minimized.

@adyach

adyach Feb 16, 2018

Member

new JSONArray().put(event).toString(2)

@antban

This comment has been minimized.

Member

antban commented Feb 16, 2018

👍

@antban

This comment has been minimized.

Member

antban commented Feb 16, 2018

deploy validation please

@adyach

This comment has been minimized.

Member

adyach commented Feb 16, 2018

👍

@antban

This comment has been minimized.

Member

antban commented Feb 19, 2018

deploy validation please

@antban

This comment has been minimized.

Member

antban commented Feb 21, 2018

deploy validation please

2 similar comments
@antban

This comment has been minimized.

Member

antban commented Feb 21, 2018

deploy validation please

@adyach

This comment has been minimized.

Member

adyach commented Feb 21, 2018

deploy validation please

@adyach

This comment has been minimized.

Member

adyach commented Mar 2, 2018

👍

@antban

This comment has been minimized.

Member

antban commented Apr 5, 2018

👍

@antban

This comment has been minimized.

Member

antban commented Apr 5, 2018

deploy validation please

@lmontrieux

This comment has been minimized.

Member

lmontrieux commented Apr 5, 2018

👍

@antban antban merged commit 934ac9f into master Apr 5, 2018

7 of 8 checks passed

Codacy/PR Quality Review Not so good... This pull request quality could be better.
Details
codecov/patch 86.84% of diff hit (target 53.18%)
Details
codecov/project 53.64% (+0.46%) compared to 34bb11f
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
default Build finished.
Details
zappr Approvals: @antban, @lmontrieux.
zappr/pr/specification PR has passed specification checks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment