-
Notifications
You must be signed in to change notification settings - Fork 365
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 1 commit
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 |
---|---|---|
|
@@ -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.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
Added deprecation message