-
Notifications
You must be signed in to change notification settings - Fork 367
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
Remove ADLS hint from import #7581
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,7 +106,6 @@ In addition, the following for provider-specific permissions may be required: | |
</ul> | ||
<div markdown="1" id="aws-s3"> | ||
|
||
|
||
## AWS S3: Importing from public buckets | ||
{:.no_toc} | ||
|
||
|
@@ -149,20 +148,11 @@ the following policy needs to be attached to the lakeFS S3 service-account to al | |
|
||
</div> | ||
<div markdown="1" id="azure-storage"> | ||
See [Azure deployment][deploy-azure-storage-account-creds] on limitations when using account credentials. | ||
|
||
### Azure Data Lake Gen2 | ||
{:.no_toc} | ||
|
||
lakeFS requires a hint in the import source URL to understand that the provided storage account is ADLS Gen2 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should document the change. Otherwise someone who's already imported has no way of understanding what went wrong! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added deprecation message |
||
**Note:** The use of the `alds` hint for ADLS Gen2 storage accounts is deprecated, please use the original source url for import. | ||
{: .note} | ||
|
||
``` | ||
For source account URL: | ||
https://<my-account>.core.windows.net/path/to/import/ | ||
|
||
Please add the *adls* subdomain to the URL as follows: | ||
https://<my-account>.adls.core.windows.net/path/to/import/ | ||
``` | ||
See [Azure deployment][deploy-azure-storage-account-creds] on limitations when using account credentials. | ||
|
||
</div> | ||
<div markdown="1" id="gcs"> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -209,7 +209,7 @@ func (a *Adapter) GetWalker(uri *url.URL) (block.Walker, error) { | |
return nil, err | ||
} | ||
|
||
return NewAzureBlobWalker(client) | ||
return NewAzureDataLakeWalker(client, false) | ||
} | ||
|
||
func (a *Adapter) GetPreSignedURL(ctx context.Context, obj block.ObjectPointer, mode block.PreSignMode) (string, time.Time, error) { | ||
|
@@ -568,11 +568,11 @@ func (a *Adapter) CompleteMultiPartUpload(ctx context.Context, obj block.ObjectP | |
|
||
func (a *Adapter) GetStorageNamespaceInfo() block.StorageNamespaceInfo { | ||
info := block.DefaultStorageNamespaceInfo(block.BlockstoreTypeAzure) | ||
info.ImportValidityRegex = `^https?://[a-z0-9_-]+\.(blob|adls)\.core\.windows\.net` // added adls for import hint validation in UI | ||
info.ImportValidityRegex = `^https?://[a-z0-9_-]+\.blob\.core\.windows\.net` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This breaks any existing import scripts. Does this mean we need a major release 🤢 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As stated in the PR description - this is a breaking change. But I believe we can avoid a major release since this affects specific users we can identify There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a WA for lakectl so it doesn't break. Effectively this is only going to impact WebUI users (I'm fine with that) |
||
info.ValidityRegex = `^https?://[a-z0-9_-]+\.blob\.core\.windows\.net` | ||
|
||
if a.chinaCloud { | ||
info.ImportValidityRegex = `^https?://[a-z0-9_-]+\.(blob|adls)\.core\.chinacloudapi\.cn` | ||
info.ImportValidityRegex = `^https?://[a-z0-9_-]+\.blob\.core\.chinacloudapi\.cn` | ||
info.ValidityRegex = `^https?://[a-z0-9_-]+\.blob\.core\.chinacloudapi\.cn` | ||
} | ||
|
||
|
@@ -598,6 +598,6 @@ func (a *Adapter) newPreSignedTime() time.Time { | |
return time.Now().UTC().Add(a.preSignedExpiry) | ||
} | ||
|
||
func (a *Adapter) GetPresignUploadPartURL(ctx context.Context, obj block.ObjectPointer, uploadID string, partNumber int) (string, error) { | ||
func (a *Adapter) GetPresignUploadPartURL(_ context.Context, _ block.ObjectPointer, _ string, _ int) (string, error) { | ||
return "", block.ErrOperationNotSupported | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to say what is happening, not only what isn't: