add `--package-mode` for new service to package #1112

msyyc · 2022-01-04T04:45:31Z

try the following cmd:

(powershell)

autorest --version=latest --python --use=D:\dev2\autorest.python `
--package-mode=dataplane  `
--output-folder=D:\dev2\azure-sdk-for-python\sdk\webpubsubexample\azure-messaging-webpubsubservice `
--input-file=https://raw.githubusercontent.com/Azure/azure-rest-api-specs/main/specification/webpubsub/data-plane/WebPubSub/stable/2021-10-01/webpubsub.json `
--credential-scopes=https://webpubsub.azure.com/.default `
--package-name=azure-messaging-webpubsubservice `
--package-pprint-name="Azure WebPubSub Service" `
--title=WebPubSubServiceClient

(linux)

autorest --version=latest --python --use=/_/dev2/autorest.python \
--package-mode=dataplane  \
--output-folder=/_/dev2/azure-sdk-for-python/sdk/webpubsubexample/azure-messaging-webpubsubservice \
--input-file=https://raw.githubusercontent.com/Azure/azure-rest-api-specs/main/specification/webpubsub/data-plane/WebPubSub/stable/2021-10-01/webpubsub.json \
--credential-scopes=https://webpubsub.azure.com/.default \
--package-name=azure-messaging-webpubsubservice \
--package-pprint-name="Azure WebPubSub Service" \
--title=WebPubSubServiceClient

…into package-data-plane-2021-12-01

msyyc · 2022-01-04T09:13:43Z

mypy autorest succeeded in Windows but failed in Linux:

iscai-msft

i think the scope of this PR is too big, really all we want to tackle right now is to add the different setup.pys that @lmazuel mentioned in his issue

iscai-msft · 2022-01-05T00:02:16Z

autorest/codegen/__init__.py

+        not all([options["package_name"], options["client_name"], options["credential_scopes"],
+                options["package_pprint_name"], options["output_folder"], options["input_file"]]):
+        raise ValueError(
+            "--package-name, --title, --package-pprint-name, --credential-scopes, --output-folder"


I don't think all of these flags need to be required for package-mode to work. For example, at least for title, we can get the value from the client name

Indeed, most of those flags will be required anyway, and we should get the values from the codemodel, once they are solved and resolved. For instance, the codemodel will always have a title.

For options that are not necessary today to generate, see that I did a proposal in the initial issue, and it was not to add new flags.

Hi @lmazuel @iscai-msft, when people want to generate a shippable package, these flags(package_name, output_folder, etc) are necessary in swagger/readme.md. Rather than that let users configure readme.md then generate package with the configuration, I think it is more convenient to do that with one step: force users to input necessary flags with 'package-mode`. After generation, users could get shippable package directly. It may be a little different with #1053, but it shall be convenient for users, too.

title: although it is usually defined in swagger, but most of it is invalid for SDK like this. So I think it is better to let users input for confirmation.

I try to let users input less parameters as soon as possible, now they only need 6.

@msyyc I'm kind of confused, wouldn't people be running --package-mode with generation, so there's no need to add duplicate flags explicitly for pakcage mode?

For example, --title shouldn't be required because that's a flag to override the client name in the swagger. Don't we want the package name's client to be the same as the generated client's name? So i don't get why you want to require --title for package mode. Honestly the only flag I think we may need to do a check on is --package-pprint-name, and I'm not sure we even have to require this flag, we could default to the client name if the flag is not specified

iscai-msft · 2022-01-05T00:04:00Z

autorest/codegen/__init__.py

@@ -289,6 +304,12 @@ def _build_code_model_options(self) -> Dict[str, Any]:
            "low_level_client": low_level_client,
            "combine_operation_files": self._autorestapi.get_boolean_value("combine-operation-files", version_tolerant),
            "python3_only": python3_only,
+            "package_mode": self._autorestapi.get_value("package-mode"),


Some of these are already in the options dict / can be get other ways. For example. client_name, credential_scopes, output_folder, and input_file

Those parameters is to render template for package-mode. I don't see where I could get them except in options.

@msyyc, we already have informnation for all of the flags you've added here, besides package-pprint-name, this is doubling code for no real reason. If you go through the code here you can see how we're already storing this information on the code model (not necessarily just in options)

iscai-msft · 2022-01-05T00:04:42Z

autorest/codegen/__init__.py

@@ -306,8 +327,16 @@ def _build_code_model_options(self) -> Dict[str, Any]:
        if azure_arm:
            options["credential"] = True
            options["head_as_boolean"] = True
+
+        if options["package_mode"] == 'dataplane':


I don't think adding these options is in the scope of this PR, not sure all of these should default to these values as well

Correct, the scope of the PR is templating, not changing behavior

The changing behavior is necessary to generate a whole package: package-mode only asks users to input limited parameter. So it is necessary to help configure some other parameters.
For example, codegen asks to input --credential if users want to use --credential-scopes. For convenience, we want users to input limited parameters. So package-mode only asks to input --credential-scopes. Then we have to help to replenish options["credential"] = True

autorest/codegen/serializers/__init__.py

autorest/codegen/templates/templates_package_dataplane/setup.py.jinja2

iscai-msft · 2022-01-05T00:13:52Z

autorest/codegen/templates/templates_package_dataplane/setup.py.jinja2

+        ]
+    ),
+    install_requires=[
+        "azure-core<2.0.0,>=1.20.1",


since we're setting minversions in multiple setup.py, we should have one common area for min versions

each mode has its own dependency so I think it is better to put them solely. And we could update the template when new autorest.python is released if necessary.

The only realy diff in mode dependency is --azure-arm requires azure-mgmt-core, whil dataplane does not. We should really share dependency requirements for evyerthing else

autorest/codegen/templates/templates_package_dataplane/setup.py.jinja2

msyyc · 2022-01-05T06:28:25Z

i think the scope of this PR is too big, really all we want to tackle right now is to add the different setup.pys that @lmazuel Laurent Mazuel FTE mentioned in his issue

Hi @iscai-msft , This PR is part of 10-minutes-shippable. Its target is to create a new usable package quickly(like Dataplane-Codegen-Quick-Start-with-tools). --package-mode is for fresh service team so it hardcodes many options to make the generation as easier as possible. It is also the requirements from Shanghai LLC Team. Maybe there is gap between our understandings. So welcome for any suggestions.

iscai-msft · 2022-01-05T23:35:33Z

@msyyc following up with @lmazuel to make sure we're on the same page, since I'm not quite sure myself what the plan is for python here. thanks!

lmazuel · 2022-01-12T20:58:12Z

autorest/codegen/__init__.py

@@ -76,6 +76,21 @@ def _validate_code_model_options(options: Dict[str, Any]) -> None:
            "If you want operation files, pass in flag --show-operations"
        )

+    if options["package_mode"] == 'dataplane' and \


The value "dataplane" should not be hardcoded, we should have a map of value to folder, and the template folder should be part of the autorest delivery

For instance, the folder autorest/codegen/packaging_template/dataplane contains the dataplane template, and the folder autorest/codegen/packaging_template/abcxyz will contain the template abcxyz

template could be distinguished with different folder:
we could make a map with that.

And different package-mode may need different parameters, so here we had better keep it.

lmazuel · 2022-01-12T20:59:07Z

autorest/codegen/__init__.py

@@ -134,7 +149,7 @@ def _create_code_model(self, yaml_data: Dict[str, Any], options: Dict[str, Union
        code_model.setup_client_input_parameters(yaml_data)

        # Get my namespace
-        namespace = self._autorestapi.get_value("namespace")
+        namespace = code_model.options.get("namespace", self._autorestapi.get_value("namespace"))


@iscai-msft why would there be a namesapce in the code_model.options?

yeah you're right there isn't any, @msyyc I don't think we need to change the code here

namesapce in the code_model.options is needed to render template for package-mode. Please see here: https://github.com/Azure/autorest.python/pull/1112/files#r794987407

To make it more clear, I could change it like here:

iscai-msft · 2022-02-01T18:02:27Z

autorest/codegen/__init__.py

@@ -134,7 +149,10 @@ def _create_code_model(self, yaml_data: Dict[str, Any], options: Dict[str, Union
        code_model.setup_client_input_parameters(yaml_data)

        # Get my namespace
-        namespace = self._autorestapi.get_value("namespace")
+        if not code_model.options.get("package_mode"):


i don't think we should change this code, we really want package mode to be as similar to generation flags as possible, I don't think it helps the users to have these slightly different flags for generation and package mode

iscai-msft · 2022-02-01T18:06:54Z

autorest/codegen/templates/templates_package_dataplane/CHANGELOG.md.jinja2

@@ -0,0 +1,6 @@
+# Release History


I think pretty much all of the files you've put down as specific to dataplane package mode can be shared across different packaging modes. All of the differences between --azure-arm and dataplane can be easily resolved in one file: we don't need duplicate templates for each different packaging mode. Not as important, but. wealso really don't need a separate folder for templates_package_dataplane either

iscai-msft · 2022-02-01T18:07:52Z

autorest/codegen/templates/templates_package_dataplane/README.md.jinja2

+- You need an [Azure subscription][azure_sub] to use this package.
+- An existing {{ package_pprint_name }} instance.
+
+#### Create with an Azure Active Directory Credential


you can't be sure the package uses AAD, you can look at the credential scope policy property / other credential properties on the code model to see what kind of credentials we use

iscai-msft · 2022-02-01T18:08:42Z

autorest/codegen/templates/templates_package_dataplane/dev_requirements.txt.jinja2

+-e ../../../tools/azure-devtools
+-e ../../../tools/azure-sdk-tools
+../../core/azure-core
+../../identity/azure-identity


we don't know if we need identity. Also we don't need to do 3.x checks anymore, I think by the time we merge this pr, we will have dropped 2.x support in autorest

iscai-msft · 2022-02-01T18:10:35Z

autorest/codegen/templates/templates_package_dataplane/swagger/README.md.jinja2

+{% for file in code_model.options['input_file'] -%}
+- {{ file }}
+{% endfor -%}
+output-folder: {{ '../' + code_model.options["package_name"].replace('-', '/') }}


I don't think we need to generate a swagger readme file as part of packaging... aren't they running the package mode from a config file already?

msyyc · 2022-02-14T07:56:14Z

After sync, will create another PR for this feature: #1154

msyyc added 7 commits December 9, 2021 17:13

add template files

3f15c4a

function complete

61187a8

update

048a290

update template for py3-only

6582e3a

Merge branch 'autorestv3' of https://github.com/Azure/autorest.python …

7c9615e

…into package-data-plane-2021-12-01

regenerate

e13b45a

pylint

0229b96

msyyc requested a review from iscai-msft January 4, 2022 09:05

iscai-msft requested changes Jan 5, 2022

View reviewed changes

lmazuel reviewed Jan 12, 2022

View reviewed changes

lmazuel mentioned this pull request Jan 19, 2022

Data plane codegen docs Azure/azure-sdk-for-python#22552

Closed

msyyc added 3 commits January 29, 2022 13:55

review

ca647ee

pylint

1be9f58

merge

c91d63c

iscai-msft reviewed Feb 1, 2022

View reviewed changes

msyyc closed this Feb 14, 2022

msyyc deleted the package-data-plane-2021-12-01 branch February 15, 2022 07:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add `--package-mode` for new service to package #1112

add `--package-mode` for new service to package #1112

msyyc commented Jan 4, 2022 •

edited

Loading

msyyc commented Jan 4, 2022

iscai-msft left a comment

iscai-msft Jan 5, 2022

lmazuel Jan 12, 2022

msyyc Jan 26, 2022

iscai-msft Feb 1, 2022 •

edited

Loading

iscai-msft Jan 5, 2022

msyyc Jan 26, 2022 •

edited

Loading

iscai-msft Feb 1, 2022

iscai-msft Jan 5, 2022

lmazuel Jan 12, 2022

msyyc Jan 29, 2022

iscai-msft Jan 5, 2022

msyyc Jan 26, 2022

iscai-msft Feb 1, 2022

msyyc commented Jan 5, 2022

iscai-msft commented Jan 5, 2022

lmazuel Jan 12, 2022

msyyc Jan 29, 2022 •

edited

Loading

lmazuel Jan 12, 2022

iscai-msft Jan 12, 2022

msyyc Jan 29, 2022 •

edited

Loading

iscai-msft Feb 1, 2022

iscai-msft Feb 1, 2022 •

edited

Loading

iscai-msft Feb 1, 2022

iscai-msft Feb 1, 2022

iscai-msft Feb 1, 2022

msyyc commented Feb 14, 2022 •

edited

Loading

add --package-mode for new service to package #1112

add --package-mode for new service to package #1112

Conversation

msyyc commented Jan 4, 2022 • edited Loading

msyyc commented Jan 4, 2022

iscai-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iscai-msft Feb 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msyyc Jan 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msyyc commented Jan 5, 2022

iscai-msft commented Jan 5, 2022

Choose a reason for hiding this comment

msyyc Jan 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msyyc Jan 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iscai-msft Feb 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msyyc commented Feb 14, 2022 • edited Loading

add `--package-mode` for new service to package #1112

add `--package-mode` for new service to package #1112

msyyc commented Jan 4, 2022 •

edited

Loading

iscai-msft Feb 1, 2022 •

edited

Loading

msyyc Jan 26, 2022 •

edited

Loading

msyyc Jan 29, 2022 •

edited

Loading

msyyc Jan 29, 2022 •

edited

Loading

iscai-msft Feb 1, 2022 •

edited

Loading

msyyc commented Feb 14, 2022 •

edited

Loading