delete: add possibility to soft-delete datasets and jobs

[mobuchowski]

First part of #1736 - ability to "soft delete" datasets and jobs - API to "hide" them which can be used from UI; the reason for adding this is process to hiding inactive datasets and jobs. This PR does not include the UI part.

The feature works by adding is_hidden flag to both datasets and jobs tables. Then, it changes jobs_view and adds datasets_view that hide rows where is_hidden flag is set to true value. This makes writing proper queries easier, since, you don't need to do this filtering manually everywhere as long as you use this view.

The soft-delete is reversed if the job or dataset is updated again - new version reverts the flag.

Signed-off-by: Maciej Obuchowski [email protected]

[pawel-big-lebowski]

This looks really great. The only inconsistency I see is that:

• findBySomething does return soft-deleted datasets,
• findAll does not return them.
  Such a behavior definitely makes here sense but can confuse other contributors.

What if we introduced includeDeleted parameter in the URL and make default values true and false for findByName/findAll respectively. ?

[codecov]

Codecov Report

Merging #2032 (17aaa8f) into main (27b54ed) will increase coverage by 0.03%.
The diff coverage is 80.48%.

@@             Coverage Diff              @@
##               main    #2032      +/-   ##
============================================
+ Coverage     75.11%   75.14%   +0.03%     
- Complexity     1020     1023       +3     
============================================
  Files           202      202              
  Lines          4798     4836      +38     
  Branches        392      392              
============================================
+ Hits           3604     3634      +30     
- Misses          754      762       +8     
  Partials        440      440              

Impacted Files	Coverage Δ
api/src/main/java/marquez/db/DatasetDao.java	98.52% <ø> (ø)
api/src/main/java/marquez/db/DatasetFieldDao.java	100.00% <ø> (ø)
...pi/src/main/java/marquez/db/DatasetVersionDao.java	95.83% <ø> (ø)
api/src/main/java/marquez/db/JobDao.java	100.00% <ø> (ø)
api/src/main/java/marquez/db/JobVersionDao.java	91.04% <ø> (ø)
...va/src/main/java/marquez/client/MarquezClient.java	57.43% <0.00%> (-1.83%)	⬇️
...java/src/main/java/marquez/client/MarquezHttp.java	81.91% <83.33%> (+0.20%)	⬆️
api/src/main/java/marquez/api/DatasetResource.java	95.00% <100.00%> (+0.76%)	⬆️
api/src/main/java/marquez/api/JobResource.java	93.10% <100.00%> (+0.79%)	⬆️
.../src/main/java/marquez/service/LineageService.java	85.32% <100.00%> (+0.85%)	⬆️

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

[mobuchowski]

@pawel-big-lebowski changed how this works. Now it all references view.

[mobuchowski]

@wslulciuc can you take a look?

Also, can you think of any additional tests this might need?

[pawel-big-lebowski]

I fell in love with testLineageWithDeletedDataset test ❤️

[pawel-big-lebowski]

That's a great test! 💯 🥇

[mobuchowski]

Next time I won't change query formatting AND query itself in one PR - it makes resolving conflicts unpleasant 😞

[julienledem]

I have two main comments:

1. if I agree that the sql reformatting with triple quotes is good, this should be done in a separate PR. Mixing refactoring/reformatting with same behavior and change of behavior in the same PR makes it difficult to review. We should avoid doing that in the future.
2. personally I would prefer we call this "delete". It is a soft delete since all the versions are maintained but it is a delete as these jobs and dataset don't exist anymore. If they do they just get undeleted the next time they run or get read/written.