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

Skip name prefixes for UC Schema when catalog_name contains current user #1830

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shreyas-goenka
Copy link
Contributor

Changes

Addresses #1779 and #1762 (comment).

Users report schema names getting too verbose unnecessarily when the catalog already contains the user name. This PR skips prefixing for schemas if the catalog already contains the short name (eg: shreyas_goenka or the user's email address (eg: [email protected]).

Tests

Unit tests.

@shreyas-goenka shreyas-goenka changed the title Skip prefixes for schema names when catalog is already namespaced to current user Skip name prefixes for UC Schema when catalog_name contains current user Oct 14, 2024
@shreyas-goenka shreyas-goenka marked this pull request as ready for review October 14, 2024 14:49
Copy link
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

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

If we roll this out as is, it will trigger recreates of dev schemas.

Can you figure out how many folks are using this today and how we can provide a ~graceful transition path? We could consider adding a setting for this behavior and default it either way. I don't particularly like the added complexity of that, though, but it's the best I can come up with on a whim.


// If the catalog is already namespaced to the current user, we don't need
// to prefix the schema name since it already falls under the user's namespace.
if containsUserIdentity(s.CatalogName, b.Config.Workspace.CurrentUser) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also check for the ShortName

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

Successfully merging this pull request may close these issues.

3 participants