[webpubsub] support AAD, Api management proxy #20533

msyyc · 2021-09-03T08:24:06Z

autorest --version=3.4.5 --python --track2 --use=D:\dev3\autorest.python --use=@autorest\[email protected] --python-sdks-folder=D:\dev3\azure-sdk-for-python\sdk --python-mode=update D:\dev3\azure-sdk-for-python\sdk\webpubsub\azure-messaging-webpubsubservice\swagger\README.md --version-tolerant

iscai-msft

overall @msyyc since a lot of this API is handwritten, generating autorest deleted a lot of the handwritten code. We don't want to delete any of the handwritten code, can you go back and add it back? thanks!

sdk/webpubsub/azure-messaging-webpubsubservice/azure/messaging/webpubsubservice/__init__.py

...ebpubsub/azure-messaging-webpubsubservice/azure/messaging/webpubsubservice/_configuration.py

sdk/webpubsub/azure-messaging-webpubsubservice/azure/messaging/webpubsubservice/_version.py

...e-messaging-webpubsubservice/azure/messaging/webpubsubservice/_web_pub_sub_service_client.py

msyyc · 2021-09-08T08:31:36Z

overall @msyyc Yuchao Yan FTE since a lot of this API is handwritten, generating autorest deleted a lot of the handwritten code. We don't want to delete any of the handwritten code, can you go back and add it back? thanks!

Got it. I add necessary code back, including from_connection_string

sdk/webpubsub/azure-messaging-webpubsubservice/README.md

...e-messaging-webpubsubservice/azure/messaging/webpubsubservice/_web_pub_sub_service_client.py

...ebpubsub/azure-messaging-webpubsubservice/azure/messaging/webpubsubservice/_configuration.py

sdk/webpubsub/azure-messaging-webpubsubservice/azure/messaging/webpubsubservice/_policies.py

...re-messaging-webpubsubservice/azure/messaging/webpubsubservice/aio/operations/_operations.py

sdk/webpubsub/azure-messaging-webpubsubservice/examples/send_messages_aad.py

sdk/webpubsub/azure-messaging-webpubsubservice/setup.py

...ebpubsub/azure-messaging-webpubsubservice/azure/messaging/webpubsubservice/_configuration.py

johanste · 2021-09-08T16:13:37Z

...e-messaging-webpubsubservice/azure/messaging/webpubsubservice/_web_pub_sub_service_client.py

+        }
+
+    @classmethod
+    def from_connection_string(cls, connection_string, **kwargs):


Rather than modifying the generated code (and thus it being susceptible to being overwritten when regenerating), we should attempt to use the patch_sdk() method in the _patch.py file to make modifications. Not needed for this release, however.

I think it makes sense. But I can't find an example what patch_sdk() should look like in Azure/azure-sdk-for-python

I add all the handwritten code into patch_sdk() to avoid changing generated code. Two problems left:
(1) aio does not have _patch.py and it will be deleted by codegen. I think we need to add it in autorest @iscai-msft
(2) do not have better ways to change __all__ in __init__ (). So every time these code will be covered.

...ebpubsub/azure-messaging-webpubsubservice/azure/messaging/webpubsubservice/_configuration.py

vicancy

Thanks for the effort! Some little comments from service team.

sdk/webpubsub/azure-messaging-webpubsubservice/README.md

...azure-messaging-webpubsubservice/azure/messaging/webpubsubservice/aio/operations/__init__.py

sdk/webpubsub/azure-messaging-webpubsubservice/examples/send_messages_aad.py

…essages_aad.py Co-authored-by: Liangying.Wei <[email protected]>

sdk/webpubsub/azure-messaging-webpubsubservice/CHANGELOG.md

sdk/webpubsub/azure-messaging-webpubsubservice/README.md

sdk/webpubsub/azure-messaging-webpubsubservice/azure/messaging/webpubsubservice/_patch.py

sdk/webpubsub/azure-messaging-webpubsubservice/setup.py

sdk/webpubsub/azure-messaging-webpubsubservice/tests/test_send_requests.py

sdk/webpubsub/azure-messaging-webpubsubservice/tests/test_smoke_async.py

...bsub/azure-messaging-webpubsubservice/examples/send_messages_connection_string_apim_proxy.py

sdk/webpubsub/azure-messaging-webpubsubservice/README.md

iscai-msft · 2021-09-17T15:08:24Z

sdk/webpubsub/azure-messaging-webpubsubservice/azure/messaging/webpubsubservice/_patch.py

+        """Create a new WebPubSubServiceClient from a connection string.
+
+        :param connection_string: Connection string
+        :type connection_string: ~str


I think ~str will render weirdly in the docs, this can be flattened into one line and just be :param str connection_string: Connection string

It is better to keep same annotation style with generated code. Anyway, I change ~str to str

sdk/webpubsub/azure-messaging-webpubsubservice/azure/messaging/webpubsubservice/aio/_patch.py

sdk/webpubsub/azure-messaging-webpubsubservice/samples/send_messages_aad_apim_proxy.py

iscai-msft · 2021-09-17T15:13:51Z

sdk/webpubsub/azure-messaging-webpubsubservice/samples/send_messages_aad_apim_proxy.py

+    exit()
+
+# Build a client through AAD
+client = WebPubSubServiceClient(credential=DefaultAzureCredential(), endpoint=endpoint, reverse_proxy_endpoint=reverse_proxy_endpoint)


@msyyc looking at this sample, I'm still not sure what a reverse proxy endpoint is, and why I should use it. Can you highlight why I would add a reverse proxy endpoint? Since this sample is the same as the AAD one, I don't know why I would add these extra lines of code. Thanks!

@iscai-msft actually I don't have rich domain knowledge to explain the usage about Api management proxy. @vicancy could you help to update the sample?

FYI Azure/azure-webpubsub#194

iscai-msft · 2021-09-17T15:16:09Z

...ubsub/azure-messaging-webpubsubservice/samples/send_messages_connection_string_apim_proxy.py

+
+# Build a client from the connection string. And for this example, we have enabled debug
+# tracing. For production code, this should be turned off. 
+client = WebPubSubServiceClient.from_connection_string(connection_string, logging_enable=True, reverse_proxy_endpoint=reverse_proxy_endpoint)


Also one thing I noticed in these samples: The actual calls you're doing with webpubsub are the same, the only difference is how to authenticate them. Do you think a better sample structure would be to have one sample with all of the different ways to authenticate (thinking something like this, where we show multiple ways to authenticate), and then having one sample that is client.send_to_all, with just one way of authenticating (either DefaultAzureCredential or AzureKeyCredential, since those are the ones that most people will use)?

Actually, I also think about that way. I hope customers could run sample file directly so the sample code should be simple asap. And for overall description about authentication, there is detailed info in README.md. So I hope to keep the sample like this format.

sdk/webpubsub/azure-messaging-webpubsubservice/tests/test_smoke.py

msyyc added 3 commits September 3, 2021 16:23

codegen

a21afb3

update test

a4d3d15

update readme.md

c3995d5

iscai-msft reviewed Sep 7, 2021

View reviewed changes

msyyc added 3 commits September 8, 2021 13:41

add test

95d910a

from connection string

090c8c1

example

a4fdbbb

msyyc requested a review from johanste September 8, 2021 07:12

Update _version.py

156eaa6

msyyc added 2 commits September 8, 2021 16:32

api management proxy

c374338

Update _web_pub_sub_service_client.py

d25309a

iscai-msft reviewed Sep 8, 2021

View reviewed changes

johanste reviewed Sep 8, 2021

View reviewed changes

...ebpubsub/azure-messaging-webpubsubservice/azure/messaging/webpubsubservice/_configuration.py Outdated Show resolved Hide resolved

johanste reviewed Sep 8, 2021

View reviewed changes

...ebpubsub/azure-messaging-webpubsubservice/azure/messaging/webpubsubservice/_configuration.py Outdated Show resolved Hide resolved

msyyc added 5 commits September 9, 2021 14:21

review

ec2ddb6

api management

bebb486

fix dependency

859ccc6

fix dependency

18ee661

apim proxy

fe01cff

msyyc self-assigned this Sep 10, 2021

msyyc added 3 commits September 13, 2021 10:50

edit code in _patch.py instead of generated code

3983c03

review

9cc207e

fix broken link

b4bfd53

msyyc marked this pull request as ready for review September 13, 2021 07:31

msyyc requested review from danieljurek, lmazuel, mayurid and scbedd as code owners September 13, 2021 07:31

msyyc requested a review from weshaggard as a code owner September 13, 2021 07:31

msyyc changed the title ~~[webpubsub] codegen~~ [webpubsub] support AAD, Api management proxy Sep 13, 2021

Update setup.py

be9026e

msyyc force-pushed the webpubsub-2021-09-03 branch from 319d416 to be9026e Compare September 13, 2021 11:55

msyyc and others added 4 commits September 14, 2021 15:46

Update dev_requirements.txt

e296dbe

fix ci

a367a37

fix ci

9f954b2

fix ci failure of windows_python27

0fd5b47

vicancy reviewed Sep 15, 2021

View reviewed changes

msyyc and others added 3 commits September 15, 2021 17:32

Update sdk/webpubsub/azure-messaging-webpubsubservice/examples/send_m…

b95cbee

…essages_aad.py Co-authored-by: Liangying.Wei <[email protected]>

Update sdk/webpubsub/azure-messaging-webpubsubservice/examples/send_m…

1ce7f49

…essages_aad.py Co-authored-by: Liangying.Wei <[email protected]>

Update README.md

d34c3a0

iscai-msft reviewed Sep 15, 2021

View reviewed changes

msyyc added 5 commits September 16, 2021 15:40

review

3dd7bea

regenerate code

8dcfcb9

fix ci

5fa9d25

fix ci

55537f7

move operation onto client

f49f435

iscai-msft reviewed Sep 17, 2021

View reviewed changes

msyyc added 3 commits September 22, 2021 15:56

review

132f1bc

Update send_messages_connection_string_apim_proxy.py

9534e09

Update send_messages_aad_apim_proxy.py

3bf7b82

msyyc closed this Oct 13, 2021

This was referenced Oct 14, 2021

[webpubsub] support AAD, Api management proxy #21235

Merged

[LLC] release for azure-messaging-webpubsubservice with LLC #21300

Closed

msyyc deleted the webpubsub-2021-09-03 branch November 25, 2021 07:42

[webpubsub] support AAD, Api management proxy #20533

[webpubsub] support AAD, Api management proxy #20533

Uh oh!

Conversation

msyyc commented Sep 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iscai-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

msyyc commented Sep 8, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

johanste Sep 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msyyc Sep 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vicancy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

msyyc commented Sep 3, 2021 •

edited

Loading

johanste Sep 8, 2021 •

edited

Loading

msyyc Sep 9, 2021 •

edited

Loading