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

Should remove the "host/" prefix of blob name when the blob container is set to "IsMultiTenant = false"? #5492

Closed
gdlcf88 opened this issue Sep 18, 2020 · 3 comments
Assignees
Labels
Milestone

Comments

@gdlcf88
Copy link
Contributor

gdlcf88 commented Sep 18, 2020

public virtual string Calculate(BlobProviderArgs args)
{
return CurrentTenant.Id == null
? $"host/{args.BlobName}"
: $"tenants/{CurrentTenant.Id.Value.ToString("D")}/{args.BlobName}";
}

protected virtual Guid? GetTenantIdOrNull()
{
if (!Configuration.IsMultiTenant)
{
return null;
}
return CurrentTenant.Id;
}

If a blob name is something, and the blob container is set to IsMultiTenant = false, the actual blob name will be host/something, but I think the blob name should be something itself.

@hikalkan
Copy link
Member

What if you later change your application to be multi-tenant? All your BLOB data become lost. I suppose I did it intentionally because of this reason.

@gdlcf88
Copy link
Contributor Author

gdlcf88 commented Sep 24, 2020

Hi @hikalkan,

I think when it is changed to be multi-tenant, developers should handle the existing BLOBs themself.

But, maybe the above need should not exist. A BLOB container with IsMultiTenant = false is usually used to store system-level BLOBs (such as certificates). If there is a container that may need multi-tenancy, it should be set IsMultiTenant = true even if there is only one tenant (host) currently.

In the current design, if a BLOB container is changed to be multi-tenant, BLOBs of any tenant are still stored in "host/", it is not expected (either before or after the change). And the essential reason is that the "non-multi-tenant" may be different from "arrange all tenants as the host".

@hikalkan
Copy link
Member

hikalkan commented Oct 6, 2020

#5510 (comment)

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 a pull request may close this issue.

3 participants