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

encodeURIComponent namespace and name in GET (column)lineage requests #2984

Conversation

MaartenHubrechts
Copy link
Contributor

@MaartenHubrechts MaartenHubrechts commented Nov 21, 2024

Problem

When a jobname contains special characters (such as +) the (column)lineage GET requests breaks because the URL used for the request isn't URL encoded. This leads to the lineage page not loading properly when clicking a job in the job pages that contains any of such special characters that break the request URL.

Solution

Use the encodeURIComponent function on both the namespace and name arguments so that special characters like + get encoded to %2B. This approach is also used in the datasets and jobs requests.

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)

@boring-cyborg boring-cyborg bot added the web label Nov 21, 2024
Copy link

boring-cyborg bot commented Nov 21, 2024

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).

Copy link

netlify bot commented Nov 21, 2024

Deploy Preview for peppy-sprite-186812 canceled.

Name Link
🔨 Latest commit 3791cca
🔍 Latest deploy log https://app.netlify.com/sites/peppy-sprite-186812/deploys/673f7e287ab6be0008c6c7f2

Signed-off-by: Maarten Hubrechts <maarten.hubrechts@hotmail.com>
@MaartenHubrechts MaartenHubrechts force-pushed the bug/fix-url-encoding-lineage-request branch from 19a4298 to 6ee88f2 Compare November 21, 2024 18:32
Signed-off-by: Maarten Hubrechts <maarten.hubrechts@hotmail.com>
@MaartenHubrechts MaartenHubrechts changed the title encodeURIComponent namespace and name in GET lineage request encodeURIComponent namespace and name in GET (column)lineage requests Nov 21, 2024
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.

Thanks for the contribution!

@phixMe phixMe enabled auto-merge (squash) November 22, 2024 02:31
@phixMe phixMe merged commit 7fb3d5e into MarquezProject:main Nov 22, 2024
15 checks passed
Copy link

boring-cyborg bot commented Nov 22, 2024

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

Copy link

codecov bot commented Nov 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.19%. Comparing base (41d9d3d) to head (3791cca).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2984   +/-   ##
=========================================
  Coverage     81.19%   81.19%           
  Complexity     1506     1506           
=========================================
  Files           268      268           
  Lines          7360     7360           
  Branches        325      325           
=========================================
  Hits           5976     5976           
  Misses         1226     1226           
  Partials        158      158           

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


🚨 Try these New Features:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants