Skip to content

Conversation

@schaudhari6254888
Copy link
Member

@schaudhari6254888 schaudhari6254888 commented Apr 4, 2023

Related command
az eventhubs namespace application-group policy remove

Description
Removing parameters metric-id, rate-limit-threshold from --throttling-policy-config object only for az eventhubs namespace application-group policy remove command.
Additionally changing throttling-policy-config name to policy, for az eventhubs namespace application-group policy remove command.
The change will release in Upcoming breaking change cycle i.e May.

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change
[Component Name 2] az command b: Add some customer-facing feature


This checklist is used to make sure that common guidelines for a pull request are followed.

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Apr 4, 2023

️✔️acr
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️acs
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️advisor
️✔️latest
️✔️3.10
️✔️3.9
️✔️ams
️✔️latest
️✔️3.10
️✔️3.9
️✔️apim
️✔️latest
️✔️3.10
️✔️3.9
️✔️appconfig
️✔️latest
️✔️3.10
️✔️3.9
️✔️appservice
️✔️latest
️✔️3.10
️✔️3.9
️✔️aro
️✔️latest
️✔️3.10
️✔️3.9
️✔️backup
️✔️latest
️✔️3.10
️✔️3.9
️✔️batch
️✔️latest
️✔️3.10
️✔️3.9
️✔️batchai
️✔️latest
️✔️3.10
️✔️3.9
️✔️billing
️✔️latest
️✔️3.10
️✔️3.9
️✔️botservice
️✔️latest
️✔️3.10
️✔️3.9
️✔️cdn
️✔️latest
️✔️3.10
️✔️3.9
️✔️cloud
️✔️latest
️✔️3.10
️✔️3.9
️✔️cognitiveservices
️✔️latest
️✔️3.10
️✔️3.9
️✔️config
️✔️latest
️✔️3.10
️✔️3.9
️✔️configure
️✔️latest
️✔️3.10
️✔️3.9
️✔️consumption
️✔️latest
️✔️3.10
️✔️3.9
️✔️container
️✔️latest
️✔️3.10
️✔️3.9
️✔️core
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.10
️✔️3.9
️✔️databoxedge
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️dla
️✔️latest
️✔️3.10
️✔️3.9
️✔️dls
️✔️latest
️✔️3.10
️✔️3.9
️✔️dms
️✔️latest
️✔️3.10
️✔️3.9
️✔️eventgrid
️✔️latest
️✔️3.10
️✔️3.9
️✔️eventhubs
️✔️latest
️✔️3.10
️✔️3.9
️✔️feedback
️✔️latest
️✔️3.10
️✔️3.9
️✔️find
️✔️latest
️✔️3.10
️✔️3.9
️✔️hdinsight
️✔️latest
️✔️3.10
️✔️3.9
️✔️identity
️✔️latest
️✔️3.10
️✔️3.9
️✔️iot
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️keyvault
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️kusto
️✔️latest
️✔️3.10
️✔️3.9
️✔️lab
️✔️latest
️✔️3.10
️✔️3.9
️✔️managedservices
️✔️latest
️✔️3.10
️✔️3.9
️✔️maps
️✔️latest
️✔️3.10
️✔️3.9
️✔️marketplaceordering
️✔️latest
️✔️3.10
️✔️3.9
️✔️monitor
️✔️latest
️✔️3.10
️✔️3.9
️✔️netappfiles
️✔️latest
️✔️3.10
️✔️3.9
️✔️network
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️policyinsights
️✔️latest
️✔️3.10
️✔️3.9
️✔️privatedns
️✔️latest
️✔️3.10
️✔️3.9
️✔️profile
️✔️latest
️✔️3.10
️✔️3.9
️✔️rdbms
️✔️latest
️✔️3.10
️✔️3.9
️✔️redis
️✔️latest
️✔️3.10
️✔️3.9
️✔️relay
️✔️latest
️✔️3.10
️✔️3.9
️✔️resource
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️role
️✔️latest
️✔️3.10
️✔️3.9
️✔️search
️✔️latest
️✔️3.10
️✔️3.9
️✔️security
️✔️latest
️✔️3.10
️✔️3.9
️✔️servicebus
️✔️latest
️✔️3.10
️✔️3.9
️✔️serviceconnector
️✔️latest
️✔️3.10
️✔️3.9
️✔️servicefabric
️✔️latest
️✔️3.10
️✔️3.9
️✔️signalr
️✔️latest
️✔️3.10
️✔️3.9
️✔️sql
️✔️latest
️✔️3.10
️✔️3.9
️✔️sqlvm
️✔️latest
️✔️3.10
️✔️3.9
️✔️storage
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.10
️✔️3.9
️✔️telemetry
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9
️✔️util
️✔️latest
️✔️3.10
️✔️3.9
️✔️vm
️✔️2018-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.10
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.10
️✔️3.9
️✔️latest
️✔️3.10
️✔️3.9

@ghost ghost added Auto-Assign Auto assign by bot Event Hubs az eventhubs labels Apr 4, 2023
@ghost ghost requested a review from evelyn-ys April 4, 2023 06:56
@yonzhan
Copy link
Collaborator

yonzhan commented Apr 4, 2023

EventHub

@ghost ghost requested a review from calvinhzy April 4, 2023 06:56
@ghost ghost assigned evelyn-ys Apr 4, 2023
@ghost ghost requested a review from yonzhan April 4, 2023 06:56
@schaudhari6254888 schaudhari6254888 changed the title [EventHub] az eventhubs namespace application-group policy remove: Adding breaking change in az eventhubs namespace application-group policy remove cmdlet [EventHub] az eventhubs namespace application-group policy remove: Add breaking change message for az eventhubs namespace application-group policy remove command Apr 4, 2023
@schaudhari6254888 schaudhari6254888 marked this pull request as ready for review April 4, 2023 09:36
@schaudhari6254888
Copy link
Member Author

Can someone please review & merge this pull request. Only contain the breaking change alert message

@evelyn-ys
Copy link
Member

If the "breaking change" you mentioned is "metric-id,rate-limit-threshold would not be required", then I don't think it breaks anything. So you can just modify the title & description with Add warning for balabala

@schaudhari6254888
Copy link
Member Author

If the "breaking change" you mentioned is "metric-id,rate-limit-threshold would not be required", then I don't think it breaks anything. So you can just modify the title & description with Add warning for balabala

Since we are using this parameters while removing the policy name previously.
but in future release i.e cli migration, we are deprecating it from remove command, So I think this is breaking change.

@evelyn-ys
Copy link
Member

When will you deprecate these parameters? The code change seems nothing to do with the breaking change...

Just FYI, CLI only allows breaking change twice a year and next breaking change window is May release

@schaudhari6254888
Copy link
Member Author

--throttling-policy-config

Inside the --throttling-policy-config object these parameters are present for removal policy.
In future these 2 parameters are not needed for deletion.
I want to do this change in this "breaking change cycle" only.

@evelyn-ys
Copy link
Member

So this PR is not related with this breaking change, right? Can you modify the PR title and description?

@schaudhari6254888 schaudhari6254888 changed the title [EventHub] az eventhubs namespace application-group policy remove: Add breaking change message for az eventhubs namespace application-group policy remove command [EventHub] Add breaking change message Notification for az eventhubs namespace application-group policy remove command Apr 11, 2023
@schaudhari6254888
Copy link
Member Author

So this PR is not related with this breaking change, right? Can you modify the PR title and description?

This PR, I am only giving Message notification.

appGroup = client.get(resource_group_name, namespace_name, application_group_name)

logger.warning(
'This operation will do removals based on policy name')
Copy link
Member

Choose a reason for hiding this comment

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

I don't think with this message customers are able to realize there will be breaking change in the future......

It just says policy name will be used for removals

Copy link
Member

Choose a reason for hiding this comment

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

If you want to notify that you won't accept metric-id,rate-limit-threshold in the future, pls say it explicitly

Copy link
Member Author

Choose a reason for hiding this comment

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

Hey @evelyn-ys , Sorry for the inconvenience.
Does the Description & warning message makes the good sense now?
Any suggestions are always welcome.

@evelyn-ys evelyn-ys changed the title [EventHub] Add breaking change message Notification for az eventhubs namespace application-group policy remove command [EventHub] az eventhubs namespace application-group policy remove: Add upcoming breaking change notification Apr 13, 2023
@evelyn-ys evelyn-ys merged commit 21fb838 into Azure:dev Apr 13, 2023
avgale pushed a commit to avgale/azure-cli that referenced this pull request Aug 24, 2023
…Add upcoming breaking change notification (Azure#26041)

* Breaking change

* updates

* CI's fixed

* Update src/azure-cli/azure/cli/command_modules/eventhubs/custom.py

Co-authored-by: Yishi Wang <[email protected]>

* Updates

* Updates

---------

Co-authored-by: Yishi Wang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Auto-Assign Auto assign by bot Event Hubs az eventhubs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants