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

[CosmosDB] Bug Fixes and Enhancements #11505

Closed
wants to merge 17 commits into from

Conversation

MehaKaushik
Copy link
Contributor

@MehaKaushik MehaKaushik commented Apr 6, 2020

Description

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:
    • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
    • the markdown help files have been regenerated using the commands listed here

@adxsdkps
Copy link
Collaborator

adxsdkps commented Apr 6, 2020

Can one of the admins verify this patch?

Copy link
Contributor

@msJinLei msJinLei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blueww and others added 4 commits April 7, 2020 15:39
* Add secret management extension

* Change build scripts for release

* Add test case

* Update changelog
* add resource types

* comment edits

* Get-AzGalleryImageVersion fix example args

Update samples to use proper argument  -GalleryImageDefinitionName not -ImageDefinitionName

* for RC2 release

* update changelog

* updated changelog

* updated changelog for upcoming release

* Fix typo

infomation→information

* PR comments has been resolved

* Updated help content

* Display ResourceManager correlationIds in error output

* close ticket function.

* For final RC

* For final RC

* Adding SensitivityRank parameter to DataClassification Set Cmdlets

* typo, unused imports. - enable severity, status tests. update properties count( since prod outage was removed). add tests for ticket status.

* properties count to 5

* count for prob class to 5

* fix get-azresource -ResourceGroupName -Name -ResourceType to use correct ApiVersion

* Complete cmdlet implementations

* fix get-azresource -ResourceGroupName -Name -ExpandProperties -ResourceType to use correct ApiVersion

* Fix Microsoft.Azure.PowerShell.Cmdlets.ManagedServices.dll.json

Azure#10826

* Fix changelog merge issue

* remove override DefaultContext

* Restore-AzSqlInstanceDatabase - fix "an" -> "a"

Examples: fix "an deleted instance" to "a deleted instance"

* Add changelog entry

* chore) update privacy notice text

* Update changelog.md per contributing.md

* chore) minor text correction

* Improve PolicyInsights error messages

* Address reviwer comments

* Add note about the version of PowerShell

* fix changelog merge error

* Minor helper file fix

* update macos to 10.14

* Add DisableAzureSqlServerActiveDirectoryOnlyAuthentication.cs file itself.

* Add DisableAzureSqlServerActiveDirectoryOnlyAuthentication.cs file itself.

* disable one test case that is unstable on linux

* remove file

* add DisableAzureSqlServerActiveDirectoryOnlyAuthentication.cs

* Create DisableAzureSqlServerActiveDirectoryOnlyAuthentication.cs

* Update ChangeLog.md

add cmdlet Disable-AzSqlServerActiveDirectoryOnlyAuthentication

* Update ChangeLog.md

* SAP AEM: add support for the new VM Extension for SAP

* Update ChangeLog.md

remove edit to this file

* Update ChangeLog.md

* Add cmdlet Disable-AzSqlServerActiveDirectoryOnlyAuthentication to upcoming section.

* Fixing the parameter name IsAzureADOnlyAuthentication and documentation

* reprotect and Update bug fixes

* add an alias to avoid breaking change

* Update the azure powershell developer guide with autorest powershell generator related materials

* Update the category.

* change log

* Update syntax in help files.

* Update C# SDK to throw Cloud Exception, Update default SKU to be ST2

* Escape single quote

* add TestGetResourceByNameAndType test for apiversion check

* updating tests and adding completer

* fix changelog merge error

* updating help files and tests

* add breaking changes

* Update files

* Update encoding to UTF8

* Add breaking changes (Azure#11329)

* add breaking changes

* Change useragent to align to Az3.0 modules

* Update Get-AzResourceLock.md

Added an example that uses the -AtScope parameter.

* Update ChangeLog.md

* Consume modified SDK clients.

* Remove unnecessary changes.

* Fix doc

Microsot→Microsoft
add dot

* Add a test case for Save-AzDeploymentScript cmdlet; update test recordings; update help messages; address reviewer comments

* Remove duplicate cmdlet names in Az.Resources.psd1

* Add change log comment

* Update Network SDK & test recordings

* add license header to ArrayExtension.cs

* Update New-AzNetworkInterface.md

Removed reference to the manual subnet entry and added code to allow retrieval of subnetID via PowerShell from Virtual Network.

This would be a best practice rather than hard-coding subnet ID entry.

* Updated error messages and help files.
- Share service provided details for 4xx HTTP status codes
- Updated help file examples to be accurate and consistent

* Update tests, tests records and help files.

* Update New-AzVmss for AutomaticRepairsPolicy issue.

* Update Compute SDK version to 33.0.0

* Updated CosmosDB Management version

* Updated TEE type name VSMEnclave to VBSEnclave

* Updated ChangeLog

* Removed the new version header from ChangeLog

* Added option of Byte encoding for New-AzDataLakeStoreItem, Add-AzDAtaLakeStoreItemContent, Get-AzDAtaLakeStoreItemContent

* Update URLs in help files

* AppGw recordings

* Updated CosmosDB Management SDK version (Azure#11351)

* Updated CosmosDB Management version

* Updated ChangeLog

* Removed the new version header from ChangeLog

Co-authored-by: Meha Kaushik <mekaushi@microsoft.com>

* [ASR] [V2A] Adding properties for VMware DR monitoring

* Update

* bump up version

* Fix the bug that RunVerioncontroller.ps1 act different under different directory.

* CredSan suppression

* Update tests file.

* Adding SensitivityRank parameter to DataClassification Set Cmdlets

* Update syntax in help files.

* Consume modified SDK clients.

* Remove unnecessary changes.

* Update tests, tests records and help files.

* Update tests file.

* Update ChangeLog.md

* Update Management.Sql package version

* Add rerun support to create pipeline run cmdlet

* Update changeLog

* Fix

* addressing review comments

* ChangeLog update

* add comment, add byte encoding for auto completion

* 1.Support customers specify min TLS version when creating cluster
2. Update New-AzHDInsightConfig.md

* Added retrying policy update

* updated paramsets and added help file

* Fix after merge

* initital commit

* Updated cmdlets with disk exclusion parameters

* Added test coverage

* Added ExcludeAllDataDisks Parameter

* Added disk exclusion tests

* updated markdown help

* updated param sets

* fixed tests

* Added unmanaged vm restore warning

* Updated Markdown Help file

* updated tests

* Update README.md

* Fixing bugs and Allowing Gremlin, Cassandra, Table api account creation

* Allow Encryption property to Target parameter of New-AzGalleryImageVersion cmdlet

Add the following parameters to New-AzDiskConfig: DiskIOPSReadOnly, DiskMBpsReadOnly, MaxSharesCount, GalleryImageReference
Fix tempDisk issue

* suppress breaking change issue from int to long

* Update Set-AzApiManagementGroup.md

GroupId is a required parameter, please add this in the example as well.

* Replace Sql client using generic rest client

* Update change log

* Fixing Cassandra schema and Mongo shard key null values

* Adding in ChangeLog

* Updated Help

* Revise according to review

* updated param help messages

* updated markdown help files with examples

* update cmdlet syntax

* Update ChangeLog.md

fix readme

* Polish changelog

* Comment out the exportchildproperties test in aliastest of testfilesystem, and unskip the adlstest.testfilesystem

* Rephrase changelog

* initial commit for afs multiple file restore

* added test for multiple file restore

* Fix error

* Added tests for multiple file restore

* Updated Markdown help file

* updated changelog

* resolving comments

* updated help file with examples

* Added user rg feature

* updated changelog

* GA Support module

* Update Az.Support.psd1

* Update ChangeLog.md

Move changelog from version 1.4 to upcoming release.

* Remove file

* Remove issue file

* "\" -> "/"

* Update URL of Resolve-AzError

Co-authored-by: Yunchi Wang <54880216+wyunchi-ms@users.noreply.github.com>
Co-authored-by: Yabo Hu <yabhu@microsoft.com>
Co-authored-by: aygoya <aygoya@microsoft.com>
Co-authored-by: Bill Sproule <william.sproule@microsoft.com>
Co-authored-by: bashahee <32364196+bashahee@users.noreply.github.com>
Co-authored-by: Yeming Liu <yeliu@microsoft.com>
Co-authored-by: Yeming Liu <Yeming.Liu@microsoft.com>
Co-authored-by: Ritvika Nagula <rinagula@microsoft.com>
Co-authored-by: Ikko Ashimine <eltociear@gmail.com>
Co-authored-by: Sapan Saxena <v-sapsax@microsoft.com>
Co-authored-by: Anthony Martin <antmarti@microsoft.com>
Co-authored-by: bashahee <bashahee@microsoft.com>
Co-authored-by: Jason Gilbertson <jagilber@users.noreply.github.com>
Co-authored-by: Filiz Topatan <lsefitopata@microsoft.com>
Co-authored-by: msJinLei <leijin@microsoft.com>
Co-authored-by: Hans De Mulder <Styxxy@users.noreply.github.com>
Co-authored-by: Sudhindra Kovalam <sukovala@microsoft.com>
Co-authored-by: Chris Eggert <cheggert@microsoft.com>
Co-authored-by: Damien Caro <dcaro@microsoft.com>
Co-authored-by: Nilambari <nilambari.deshpande@hotmail.com>
Co-authored-by: Nilambari <nilamd@microsoft.com>
Co-authored-by: Amol Agarwal <amagarwa@microsoft.com>
Co-authored-by: Amol Agarwal <57109831+amolagar5@users.noreply.github.com>
Co-authored-by: MSSedusch <sedusch@microsoft.com>
Co-authored-by: ayfathim <ayfathim@microsoft.com>
Co-authored-by: Xiaogang Ding <xidi@microsoft.com>
Co-authored-by: Teng Lu <telu@microsoft.com>
Co-authored-by: shahbj79 <shahbj79@hotmail.com>
Co-authored-by: Bhavin Shah <bhshah@microsoft.com>
Co-authored-by: Peter Lorenzen <theheatDK@users.noreply.github.com>
Co-authored-by: erich-wang <eriwan@microsoft.com>
Co-authored-by: Anton Evseev <v-anevse@microsoft.com>
Co-authored-by: Thomas Thornton <t.thornton@kainos.com>
Co-authored-by: gkostal <gkostal@microsoft.com>
Co-authored-by: Hyonho Lee <Hyonho.Lee@microsoft.com>
Co-authored-by: Meha Kaushik <mekaushi@microsoft.com>
Co-authored-by: Rahul Dutta <rahuldutta90@users.noreply.github.com>
Co-authored-by: Meha Kaushik <kaushik.meha@gmail.com>
Co-authored-by: Vidyadhari Jami <vijami@microsoft.com>
Co-authored-by: wyunchi-ms <wang935863431@126.com>
Co-authored-by: lijzha <lijzha@microsoft.com>
Co-authored-by: Jin Lei <54836179+msJinLei@users.noreply.github.com>
Co-authored-by: Zhenyu Zhou <zhezhou@microsoft.com>
Co-authored-by: sambitratha <es15btech11015@iith.ac.in>
Co-authored-by: Sambit Rath <sarath@microsoft.com>
Co-authored-by: erwinkramer <r3verse@gmail.com>
Co-authored-by: Yeming Liu <felix_liu@outlook.com>
Copy link

@plzm plzm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In NewAzCosmosDBAccount.cs and UpdateAzCosmosDBAccount.cs

https://github.com/Azure/azure-powershell/blob/master/src/CosmosDB/CosmosDB/CosmosDBAccount/NewAzCosmosDBAccount.cs#L191 through line 203

https://github.com/Azure/azure-powershell/blob/master/src/CosmosDB/CosmosDB/CosmosDBAccount/UpdateAzCosmosDBAccount.cs#L175 through line 187

I suggest handling the IP filter range like this, replacing the above blocks within the check that IpRangeFilter was non-null (i.e. the following only applies when IpRangeFilter was actually passed).

string IpRangeFilterAsString = IpRangeFilter?.Aggregate(string.Empty, (output, next) => string.Concat(output, (!string.IsNullOrWhiteSpace(output) && !string.IsNullOrWhiteSpace(next) ? "," : string.Empty), next)) ?? string.Empty;

Reasons:

  1. This is one line, not a block that has different code for [0] and indexes > 0.
  2. IpRangeFilterAsString will always be non-null this way. In the current implementation and this PR, IpRangeFilterAsString can still be null if a non-null but empty IpRangeFilter is passed, implying any existing rules should be removed. Would it make more sense to pass an empty string, i.e. not null, to back end in this case? The above code does that.

@MehaKaushik
Copy link
Contributor Author

In NewAzCosmosDBAccount.cs and UpdateAzCosmosDBAccount.cs

https://github.com/Azure/azure-powershell/blob/master/src/CosmosDB/CosmosDB/CosmosDBAccount/NewAzCosmosDBAccount.cs#L191 through line 203

https://github.com/Azure/azure-powershell/blob/master/src/CosmosDB/CosmosDB/CosmosDBAccount/UpdateAzCosmosDBAccount.cs#L175 through line 187

I suggest handling the IP filter range like this, replacing the above blocks within the check that IpRangeFilter was non-null (i.e. the following only applies when IpRangeFilter was actually passed).

string IpRangeFilterAsString = IpRangeFilter?.Aggregate(string.Empty, (output, next) => string.Concat(output, (!string.IsNullOrWhiteSpace(output) && !string.IsNullOrWhiteSpace(next) ? "," : string.Empty), next)) ?? string.Empty;

Reasons:

  1. This is one line, not a block that has different code for [0] and indexes > 0.
  2. IpRangeFilterAsString will always be non-null this way. In the current implementation and this PR, IpRangeFilterAsString can still be null if a non-null but empty IpRangeFilter is passed, implying any existing rules should be removed. Would it make more sense to pass an empty string, i.e. not null, to back end in this case? The above code does that.

Thanks for the suggestion, addressed it.

@MehaKaushik
Copy link
Contributor Author

@msJinLei
Addressed your comments, rebasing with latest master resulted in showing the diff of commits merged in master by other teams.

@msJinLei
Copy link
Contributor

msJinLei commented Apr 8, 2020

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@MehaKaushik
Copy link
Contributor Author

Moving to PR #11513

@MehaKaushik MehaKaushik closed this Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment