[AL-1356] Dataset Renaming #1538
Conversation
FayazRahman
requested review from
farizrahman4u and
AbhinavTuli
and removed request for
farizrahman4u
Codecov Report
@@ Coverage Diff @@
## main #1538 +/- ##
==========================================
+ Coverage 92.23% 92.25% +0.01%
==========================================
Files 194 194
Lines 17190 17277 +87
==========================================
+ Hits 15856 15939 +83
- Misses 1334 1338 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -262,6 +262,39 @@ def load( | |||
| except AgreementError as e: | |||
| raise e from None | |||
|
|
|||
| @staticmethod | |||
| def rename( | |||
There was a problem hiding this comment.
path and name are confusing. Original path and new path or original name and new name might be better
| creds = {} | ||
|
|
||
| feature_report_path(path, "rename", {}) | ||
|
|
There was a problem hiding this comment.
we should check the path, before all of this.
There was a problem hiding this comment.
checking, should include multiple things, like checking that rename is happening from s3 to s3, gcs to gcs, hub to hub etc + for hub datasets we shouldn't allow renaming with username/xyz/ds
There was a problem hiding this comment.
these are checked in the rename method of datasets
| if new_bucket != self.client_bucket.name: | ||
| raise RenameError | ||
| blob_objects = self.client_bucket.list_blobs(prefix=self.path) | ||
| for blob in blob_objects: |
There was a problem hiding this comment.
this is happening serially, can't we utilize the multi-worker copy that you had built? Or is there some difference?
| @@ -330,6 +330,31 @@ def clear(self): | |||
| else: | |||
| super().clear() | |||
|
|
|||
| def rename(self, root): | |||
| """Rename root folder""" | |||
There was a problem hiding this comment.
same question as in gcs provider
🚀 🚀 Pull Request
Checklist:
coverage-rateupChanges