Implement Azure service bus subscription (create, delete)Operators #24625
Conversation
boring-cyborg
bot
added
area:providers
kind:documentation
provider:microsoft-azure
|
Created PR for Azure service bus subscription(create, delete) operator |
bharanidharan14
force-pushed
the
asb-subscription-mgmt-operators
branch
5 times, most recently
from
aadd8f7 to
7e7a016
Compare
| @@ -114,6 +119,93 @@ def delete_queue(self, queue_name: str) -> None: | |||
| with self.get_conn() as service_mgmt_conn: | |||
| service_mgmt_conn.delete_queue(queue_name) | |||
|
|
|||
| def create_subscription( | |||
There was a problem hiding this comment.
I suggest to just use the underlying client method rather than adding this one
There was a problem hiding this comment.
@dstandish You are suggesting to call the create_subscription client method in operator ? if yes how will I get the connection ? should I call the get_conn() method in operator ?
There was a problem hiding this comment.
yes that was the suggestion. but i don't have strong feelings if you want to go this way and you've already got an approval so it's up to you.
i am just of the philosophy that when the client methods already do exactly what we need, we don't need to implement those exact methods. mainly it's for autocomplete i guess? to me it seems simpler to just use the methods and let the hook be lighter weight and reduce our maintenance burden. on the other hand, i understand there's an argument that implementing methods like this aids in discoverability of features for users, and is worth it despite adding code that doesn "need" to be there.
related note, for AWS we have a good type stubs library that we use instead of doing this. in other cases i have seen use of Protocol classes to add autocomplete.
anyway, up to you as far as i'm concerned. lemme know your thoughts.
There was a problem hiding this comment.
I agree with your point instead adding method in hook its better to add in operator, it will be lighter weight. But my only doubt can I use client methods in operator, and call the get_conn() method in operator, no where I can seeing in the repo using client method directly in operator
There was a problem hiding this comment.
You can do that, it's no problem at all. It is done for sure.
There was a problem hiding this comment.
will do that and raise the PR
There was a problem hiding this comment.
@dstandish I have addressed your suggestion, can you please review and approve it
|
@dstandish Need your review on this comment #24625 (comment) |
|
@mik-laj @dstandish Need your review on this PR |
I replied to you |
Implement Azure service bus subscription Operators Implement Azure service bus subscription Operators Implement Azure service bus subscription Operators Fix doc string Implement Azure service bus subscription Operators - Added AzureServiceBusSubscriptionCreateOperator - Added AzureServiceBusSubscriptionDeleteOperator -example DAG - Added hooks for creating subscription and delete subscription - Added unit Test case Doc string fix Remove hook method and used client method in operator
bharanidharan14
force-pushed
the
asb-subscription-mgmt-operators
branch
from
b785d4c to
011fdf6
Compare
cc: @kaxil