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

Define DbRetentionJob(Jdbi, DbRetentionConfig) #2549

Merged
merged 6 commits into from
Jul 24, 2023

Conversation

wslulciuc
Copy link
Member

@wslulciuc wslulciuc commented Jul 24, 2023

Problem

Validate DbRetentionConfig properties internally within class. That is, when instantiating a new DbRetentionConfig object, the caller should expect all config values to be valid, contain a reasonable default (if not set), and be immutable. The caller should not be expected to validate the properties externally from the class.

Solution

Add @Positive to DbRetentionConfig instance variables with accompanying test suite.

Checklist

  • You've signed-off your work
  • Your changes are accompanied by tests (if relevant)
  • Your change contains a small diff and is self-contained
  • You've updated any relevant documentation (if relevant)
  • You've included a one-line summary of your change for the CHANGELOG.md (Depending on the change, this may not be necessary).
  • You've versioned your .sql database schema migration according to Flyway's naming convention (if relevant)
  • You've included a header in any source code files (if relevant)

Signed-off-by: wslulciuc <willy@datakin.com>
Signed-off-by: wslulciuc <willy@datakin.com>
Signed-off-by: wslulciuc <willy@datakin.com>
Signed-off-by: wslulciuc <willy@datakin.com>
@boring-cyborg boring-cyborg bot added the api API layer changes label Jul 24, 2023
Signed-off-by: wslulciuc <willy@datakin.com>
@wslulciuc wslulciuc requested a review from phixMe July 24, 2023 14:30
Signed-off-by: wslulciuc <willy@datakin.com>
@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Merging #2549 (1cfeb9e) into main (a515c91) will increase coverage by 0.15%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               main    #2549      +/-   ##
============================================
+ Coverage     83.15%   83.30%   +0.15%     
  Complexity     1286     1286              
============================================
  Files           244      243       -1     
  Lines          5942     5931      -11     
  Branches        282      279       -3     
============================================
  Hits           4941     4941              
+ Misses          854      843      -11     
  Partials        147      147              
Impacted Files Coverage Δ
api/src/main/java/marquez/MarquezApp.java 69.04% <0.00%> (+3.13%) ⬆️
api/src/main/java/marquez/jobs/DbRetentionJob.java 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@phixMe phixMe left a comment

Choose a reason for hiding this comment

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

Nice validation, Thanks for this.

@wslulciuc wslulciuc merged commit 3aa0b5b into main Jul 24, 2023
@wslulciuc wslulciuc deleted the feature/follow-up-db-retention branch July 24, 2023 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api API layer changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants