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-1680: switched to strict validation; removed feature toggle; #888

Merged
merged 4 commits into from Jun 13, 2018

Conversation

3 participants
@v-stepanov
Member

v-stepanov commented Jun 11, 2018

Switch to strict mode

Zalando ticket : ARUHA-1680

Description

Real switch to strict validation

Review

  • Tests
  • Documentation
  • CHANGELOG

Deployment Notes

Deploy and pray. (But actually we were running in shadow mode live for almost 3 days and there was no a single problem)

@codecov-io

This comment has been minimized.

codecov-io commented Jun 11, 2018

Codecov Report

Merging #888 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #888      +/-   ##
============================================
+ Coverage     54.04%   54.05%   +<.01%     
+ Complexity     1750     1749       -1     
============================================
  Files           315      315              
  Lines          9659     9650       -9     
  Branches        888      886       -2     
============================================
- Hits           5220     5216       -4     
+ Misses         4116     4112       -4     
+ Partials        323      322       -1
Impacted Files Coverage Δ Complexity Δ
...g/zalando/nakadi/service/FeatureToggleService.java 51.61% <ø> (-1.52%) 0 <0> (ø)
...va/org/zalando/nakadi/domain/StrictJsonParser.java 80.53% <100%> (+0.67%) 43 <2> (+1) ⬆️
...main/java/org/zalando/nakadi/domain/BatchItem.java 85.14% <100%> (+3%) 24 <0> (-1) ⬇️
...ava/org/zalando/nakadi/service/EventPublisher.java 91.57% <100%> (-0.09%) 20 <0> (ø)
...n/java/org/zalando/nakadi/domain/BatchFactory.java 85.39% <100%> (-0.17%) 52 <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 0d77122...16684d6. Read the comment docs.

} catch (final JSONException e) {
// temporary logging
LOG.debug("[STRICT_JSON_FAIL] Failed to parse event with strict parser: {} Error message: {}",
rawEvent, e.getMessage());

This comment has been minimized.

@antban

antban Jun 11, 2018

Member

@v-stepanov I think that this message should be either logged in StrictJsonParser, either on the top level with exception.

@antban

This comment has been minimized.

Member

antban commented Jun 11, 2018

I have only 1 doubt about logging place. Everything else is fine

@antban

This comment has been minimized.

Member

antban commented Jun 11, 2018

👍

@v-stepanov

This comment has been minimized.

Member

v-stepanov commented Jun 11, 2018

deploy validation please

@antban

This comment has been minimized.

Member

antban commented Jun 13, 2018

👍

1 similar comment
@v-stepanov

This comment has been minimized.

Member

v-stepanov commented Jun 13, 2018

👍

@v-stepanov v-stepanov merged commit 857c84c into master Jun 13, 2018

7 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
codecov/patch 100% of diff hit (target 54.04%)
Details
codecov/project 54.05% (+<.01%) compared to 0d77122
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
zappr Approvals: @antban, @v-stepanov.
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