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

Web: Created by view for dataset versions #1929

Merged
merged 2 commits into from
May 3, 2022
Merged

Conversation

phixMe
Copy link
Member

@phixMe phixMe commented Mar 30, 2022

Signed-off-by: Peter Hicks [email protected]

Problem

Some Marquez users were eager to see the run that created a dataset version.

Solution

We've added the Created by Run section on the dataset page as well as the dataset version list view. Also updated is syntax highlighting support for sql contexts and the empty states for context that is missing for us.
image
image
image
image

Note: All database schema changes require discussion. Please link the issue for context.

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)

@codecov
Copy link

codecov bot commented Mar 30, 2022

Codecov Report

Merging #1929 (b72eb3c) into main (1e3110e) will decrease coverage by 0.33%.
The diff coverage is n/a.

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

@@             Coverage Diff              @@
##               main    #1929      +/-   ##
============================================
- Coverage     78.23%   77.90%   -0.34%     
+ Complexity      955      937      -18     
============================================
  Files           194      193       -1     
  Lines          5293     5218      -75     
  Branches        420      418       -2     
============================================
- Hits           4141     4065      -76     
  Misses          706      706              
- Partials        446      447       +1     
Impacted Files Coverage Δ
...ain/java/marquez/db/mappers/DatasetDataMapper.java 76.92% <0.00%> (-8.27%) ⬇️
.../src/main/java/marquez/service/models/Dataset.java 69.23% <0.00%> (-3.19%) ⬇️
...in/java/marquez/service/models/DatasetVersion.java 64.00% <0.00%> (-2.67%) ⬇️
api/src/main/java/marquez/MarquezApp.java 62.31% <0.00%> (-2.55%) ⬇️
api/src/main/java/marquez/db/DbMigration.java 34.37% <0.00%> (-1.99%) ⬇️
...main/java/marquez/service/models/LineageEvent.java 74.54% <0.00%> (-1.73%) ⬇️
...va/marquez/db/mappers/DatasetVersionRowMapper.java 87.50% <0.00%> (-1.39%) ⬇️
...rc/main/java/marquez/db/mappers/DatasetMapper.java 81.48% <0.00%> (-1.28%) ⬇️
...src/main/java/marquez/db/mappers/JobRowMapper.java 70.83% <0.00%> (-1.17%) ⬇️
...ez/db/mappers/ExtendedDatasetVersionRowMapper.java 90.00% <0.00%> (-0.91%) ⬇️
... and 19 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@phixMe phixMe added feature web javascript Pull requests that update Javascript code labels Mar 30, 2022
@phixMe phixMe marked this pull request as ready for review March 30, 2022 20:06
@phixMe phixMe added the review Ready for review label Mar 30, 2022
@wslulciuc wslulciuc added this to the 0.22.0 milestone Mar 31, 2022
@wslulciuc
Copy link
Member

@phixMe, love to see the progress on this! I'm wondering, can we simplify the change by just linking the run that created the dataset? That is, in the version history tab, can we just have the runID be "clickable" and link out to the run metadata? I like the idea of showing the state of the run, but a dataset version is created on a successful job run, so it'd always be "green" (=complete run state). So, I think just the runID would be enough.

In the latest schema tab, I would also just display the runID (maybe on the top right?) as well and make that clickable. Given that the run metadata comes after the fact metadata, the run metadata would be far down the page if the run contained a large fact value.

@wslulciuc
Copy link
Member

@phixMe: To not block the merge, let's address any feedback as follow up work

Copy link
Member

@wslulciuc wslulciuc left a comment

Choose a reason for hiding this comment

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

LGTM 🛳️

@wslulciuc wslulciuc enabled auto-merge (squash) May 3, 2022 18:06
@wslulciuc wslulciuc removed the review Ready for review label May 3, 2022
@wslulciuc wslulciuc merged commit 276ae77 into main May 3, 2022
@wslulciuc wslulciuc deleted the feature/web-dataset-run branch May 3, 2022 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature javascript Pull requests that update Javascript code web
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants