Skip to content

version history: deduplicate the versions displayed#2854

Merged
phixMe merged 3 commits into
MarquezProject:mainfrom
namyyys:proposal/change-version-history-display
Jul 22, 2024
Merged

version history: deduplicate the versions displayed#2854
phixMe merged 3 commits into
MarquezProject:mainfrom
namyyys:proposal/change-version-history-display

Conversation

@namyyys

@namyyys namyyys commented Jul 17, 2024

Copy link
Copy Markdown
Contributor

Problem

In the version history, a dataset will sometimes have the same version displayed multiple times.
Capture d’écran 2024-07-16 à 10 55 07

After investigating, i found out that this comes from the join clause between the dataset_versions and datasets_view in this function. The symlinks are included in the datasets_view using the same uuid as the dataset and so are displayed in the same logic as the primary dataset.

Solution

  • Exclude the symlinks from the result of the query to display the version history
  • this will be done by creating a CTE 'dataset_symlinks_names' with all the symlinks name. Then we check that all the datasets name included in the list of version does not have a symlink name (and so is not a symlink)

For the same dataset in the above example we have now this display
Capture d’écran 2024-07-16 à 11 14 47

One-line summary:

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: Namizata <namizata.sangare@decathlon.com>
@boring-cyborg boring-cyborg Bot added the api API layer changes label Jul 17, 2024
@boring-cyborg

boring-cyborg Bot commented Jul 17, 2024

Copy link
Copy Markdown

Thanks for opening your first pull request in the Marquez project! Please check out our contributing guidelines (https://github.com/MarquezProject/marquez/blob/main/CONTRIBUTING.md).

@netlify

netlify Bot commented Jul 17, 2024

Copy link
Copy Markdown

Deploy Preview for peppy-sprite-186812 canceled.

Name Link
🔨 Latest commit 52d183f
🔍 Latest deploy log https://app.netlify.com/sites/peppy-sprite-186812/deploys/669e96e232fa690008c6bb12

@codecov

codecov Bot commented Jul 17, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.74%. Comparing base (8010387) to head (826ed70).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2854   +/-   ##
=========================================
  Coverage     84.74%   84.74%           
  Complexity     1456     1456           
=========================================
  Files           253      253           
  Lines          6562     6562           
  Branches        305      305           
=========================================
  Hits           5561     5561           
  Misses          850      850           
  Partials        151      151           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

.build()))
.outputs(Collections.emptyList())
.build();
System.out.println("event is " + lineageEvent);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remove log statement here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i pushed a new version removing the log statement

@phixMe phixMe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the change, happy to merge after the log is removed!

Signed-off-by: Namizata <namizata.sangare@decathlon.com>

@phixMe phixMe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the fix! 💯 👍

@phixMe phixMe enabled auto-merge (squash) July 22, 2024 16:51
@phixMe phixMe merged commit f0b2195 into MarquezProject:main Jul 22, 2024
@boring-cyborg

boring-cyborg Bot commented Jul 22, 2024

Copy link
Copy Markdown

Great job! Congrats on your first merged pull request in the Marquez project!

jonathanpmoraes referenced this pull request in nubank/NuMarquez Feb 6, 2025
* version history: deduplicate the versions displayed

Signed-off-by: Namizata <namizata.sangare@decathlon.com>

* remove log from test

Signed-off-by: Namizata <namizata.sangare@decathlon.com>

---------

Signed-off-by: Namizata <namizata.sangare@decathlon.com>
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