fix(browse): Fixing browse path to remove requirement for simple name suffix #5634
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…j--fix-browse-name-split
…j--fix-browse-name-split
…j--fix-browse-name-split
jjoyce0510
changed the title
…j--fix-browse-name-split
…j--fix-browse-name-split
…j--fix-browse-name-split
…j--fix-browse-name-split
…j--fix-browse-name-split
…j--fix-browse-name-split
…j--fix-browse-name-split
…j--fix-browse-name-split
gabe-lyons
reviewed
Aug 16, 2022
| ## Support | ||
|
|
||
| The Acryl team will be on standby to assist you in your migration. Please | ||
| join [#release-0_8_0](https://datahubspace.slack.com/archives/C0244FHMHJQ) channel and reach out to us if you find |
There was a problem hiding this comment.
looks like this channel has been deleted. should we just pick one that already exists?
gabe-lyons
reviewed
Aug 16, 2022
| case Constants.CHART_ENTITY_NAME: | ||
| ChartKey chartKey = (ChartKey) EntityKeyUtils.convertUrnToEntityKey(urn, getKeySchema(urn.getEntityType(), entityRegistry)); | ||
| return ("/" + chartKey.getDashboardTool()); | ||
| case Constants.DASHBOARD_ENTITY_NAME: // TODO -> Improve the quality of our browse path here. |
There was a problem hiding this comment.
i think the answer here might lie in providing a container to dashboards, data flows and datajobs rather than improve the urn-based browse path generation
There was a problem hiding this comment.
Yeah absolutely - we need containers for these.
5 tasks
gabe-lyons
approved these changes
Aug 19, 2022
shirshanka
pushed a commit
to shirshanka/datahub
that referenced
this pull request
Sep 8, 2022
shirshanka
pushed a commit
to shirshanka/datahub
that referenced
this pull request
Sep 8, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
For historical reasons, browse paths have always required to end with a "simple name". For example,
/my/custom/path/SimpleName.In the UI, we've been hiding the final path component, and instead redirecting users directly to the entity which is associated with the SimpleName.
We've recently classified this legacy behavior as a bug in DataHub which needs to be addressed in order for browse to work as users expect. Users expect to be able to provide a simple path which omits the final "simple name", for example:
/my/custom/pathand simply be able to view the entity under that path. This PR adds exactly that capability, while also solving a longstanding known issue where a single entity cannot have > 1 browse path.The implications are that existing browse paths will no longer behave as the previously have, requiring a single additional click to navigate to any given entity. In order to address this, we've included an optional upgrade which can be configured using a runtime environment variable on the
datahub-gmspod:UPGRADE_DEFAULT_BROWSE_PATHS_ENABLED=trueA more detailed description is coming in a dedicated doc that will describe the issue and our solving of the issue.
Status
Ready for review.
Checklist