Deprecated new_name in Undelete#18690
Conversation
tasherif-msft
requested review from
amishra-dev,
annatisch,
kasobol-msft,
xiafu-msft and
zezha-msft
as code owners
ghost
added
the
amishra-dev
left a comment
amishra-dev
left a comment
There was a problem hiding this comment.
Can we update the API comments to indicate that the parameter does not work if the value is different than the original container?
So we want to keep the parameter even though the only applicable value is if it matches the old one? |
isnt' that a breaking change? |
Well technically in Python they can still pass it if they want to (since it was a kwarg not a regular argument) and currently we ARE parsing it like we did before - its just now we surface a warning (if the cx passes it) and let service throw accordingly. However since this kwarg is no longer supported thought it would be best to get removed from the docstrings as an accepted kwarg. Do you feel like it should be kept in the docstrings as an accepted kwarg and add a deprecation comment besides it? (we don't believe that's necessary) |
| """ | ||
| new_name = kwargs.pop('new_name', None) | ||
| if new_name: | ||
| warnings.simplefilter('always', DeprecationWarning) |
There was a problem hiding this comment.
Hi Anna! I think we should let the user to choose how they want to deal with the warning, what do you think? @annatisch
| async def test_restore_to_existing_file_system(self, datalake_storage_account_name, datalake_storage_account_key): | ||
| # Needs soft delete enabled account. | ||
| if not self.is_playback(): | ||
| return | ||
| self._setUp(datalake_storage_account_name, datalake_storage_account_key) | ||
| # get an existing filesystem | ||
| existing_name = self._get_file_system_reference(prefix="existing") | ||
| name = self._get_file_system_reference(prefix="filesystem") | ||
| existing_filesystem_client = await self.dsc.create_file_system(existing_name) | ||
| filesystem_client = await self.dsc.create_file_system(name) | ||
|
|
||
| # Act | ||
| await filesystem_client.delete_file_system() | ||
| # to make sure the filesystem deleted | ||
| with self.assertRaises(ResourceNotFoundError): | ||
| await filesystem_client.get_file_system_properties() | ||
|
|
||
| filesystem_list = [] | ||
| async for fs in self.dsc.list_file_systems(include_deleted=True): | ||
| filesystem_list.append(fs) | ||
| self.assertTrue(len(filesystem_list) >= 1) | ||
|
|
||
| for filesystem in filesystem_list: | ||
| # find the deleted filesystem and restore it | ||
| if filesystem.deleted and filesystem.name == filesystem_client.file_system_name: | ||
| with self.assertRaises(HttpResponseError): | ||
| await self.dsc.undelete_file_system(filesystem.name, filesystem.deleted_version, | ||
| new_name=existing_filesystem_client.file_system_name) | ||
|
|
There was a problem hiding this comment.
I think we still need to test restoring to existing filesystem/container, while it can be added later and tracked in an issue
the steps are
- use soft delete enabled account
- create container A and soft delete it
- recreate container A
- restore deleted container and see if we can get HttpResponseError
Add new enum values to DataBindingMode (Azure#18690)
3 participants
new_nameis no longer supported on the service side forundelete_container()andundelete_filesystem()