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

Ensure job data in lineage query is not null or empty #2253

Merged
merged 9 commits into from
Dec 13, 2022

Conversation

wslulciuc
Copy link
Member

@wslulciuc wslulciuc commented Nov 18, 2022

Problem

An unknown edge case in our lineageAPI exists when querying for a job uuid that has no lineage when calling LineageDao.getLineage(), yet is associated with a given dataset D. That is, given a dataset D, a reverse job lookup is performed using LineageDao.getJobFromInputOrOutput() (which returns the job uuid that produced dataset D) but when querying for the lineage data for the returned job an empty set is returned, therefore resulting in a backend exception (=5xx status code) when invoking LineageDao.getJobFromInputOrOutput().

Solution

This PR ensures the lineage data for a job is not empty (an empty lineage graph is returned instead) and introduces logs to better understand the scenario in which this unknown edge case occurs for debugging purposes. This PR also adds a API check to ensure the query param nodeID provided in the lineageAPI call exists.

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 updated the CHANGELOG.md with details about your change under the "Unreleased" section (if relevant, 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)

@boring-cyborg boring-cyborg bot added the api API layer changes label Nov 18, 2022
@codecov
Copy link

codecov bot commented Nov 18, 2022

Codecov Report

Merging #2253 (5e4aeea) into main (3212c8f) will decrease coverage by 0.07%.
The diff coverage is 61.11%.

❗ Current head 5e4aeea differs from pull request most recent head 5cd3207. Consider uploading reports for the commit 5cd3207 to get more accurate results

@@             Coverage Diff              @@
##               main    #2253      +/-   ##
============================================
- Coverage     77.07%   76.99%   -0.08%     
- Complexity     1170     1171       +1     
============================================
  Files           222      222              
  Lines          5317     5326       +9     
  Branches        425      426       +1     
============================================
+ Hits           4098     4101       +3     
- Misses          747      751       +4     
- Partials        472      474       +2     
Impacted Files Coverage Δ
.../src/main/java/marquez/service/LineageService.java 83.07% <61.11%> (-3.70%) ⬇️

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

@wslulciuc wslulciuc marked this pull request as ready for review December 5, 2022 15:29
@wslulciuc wslulciuc added the review Ready for review label Dec 5, 2022
Copy link
Collaborator

@collado-mike collado-mike left a comment

Choose a reason for hiding this comment

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

Nice! 🥳

@wslulciuc wslulciuc enabled auto-merge (squash) December 12, 2022 23:01
@wslulciuc wslulciuc merged commit e11a792 into main Dec 13, 2022
@wslulciuc wslulciuc deleted the bug/bind-list-error-lineage-dao branch December 13, 2022 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api API layer changes review Ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants