feat: generate snippet metadata #1129

busunkim96 · 2022-01-11T21:07:14Z

Follow up to #1121.

Main changes:

When snippets are generated, snippet metadata is also generated.
If a method has a snippet, that snippet is included in the method docstring.

Other changes:

Removed the method docstring in the code snippet file (line right below the method definition) since the same text is already in the comment block at the top of the file.
Removed the concept of a "standalone" sample. All generated samples are expected to be standalone. When someone wants a smaller portion of the sample (e.g., request initialization only) they should fetch it from the file by looking up the line numbers in the snippet metadata file.

Other Notes:

~~It doesn't look like it's possible to do type annotations with _pb2 types, so those are annotated as Any.~~ It is possible to do mypy checking with https://github.com/dropbox/mypy-protobuf, but I think it will be easier make that change in a separate PR.
There are a lot of golden file updates, this range of commits has most of the generator and test changes.

Use snippet metadata proto to add generated snippets into method docstrings. Metadata generation is intentionally incomplete, the fields that are filled out are a minimal set to enable snippets in docstrings. The proto itself will move elsewhere, but it does not yet have a permanent home.

…generator-python into snippets-in-docstrings t push

…generator-python into snippets-in-docstrings

software-dov

Looks great! Just a few inline questions, no need to block submission.

software-dov · 2022-01-13T22:12:07Z

gapic/generator/generator.py

-        # output_files[manifest_fname] = CodeGeneratorResponse.File(
-        #     content=manifest_doc.render(), name=manifest_fname
-        # )
+        if len(index.metadata_index.snippets) > 0:


Nit:

if index.metadata_index.snippets:

software-dov · 2022-01-13T22:13:01Z

gapic/generator/generator.py

+            # NOTE(busunkim): Not all fields are yet populated in the snippet metadata.
+            # Expected filename: snippet_metadata_{metadata_schema_version}_{apishortname}_{apiversion}.json
+            snippet_metadata_path = str(pathlib.Path(
+                out_dir) / f"snippet_metadata_v1_{api_schema.naming.name}_{api_schema.naming.version}.json").lower()


To clarify: the v1 here is about the snippet metadata itself, correct?

Yes! Amanda left a note below saying the metadata will not be versioned for now though, so I'll go ahead and remove it.

software-dov · 2022-01-13T22:13:47Z

gapic/generator/generator.py

+            # NOTE(busunkim): Not all fields are yet populated in the snippet metadata.
+            # Expected filename: snippet_metadata_{metadata_schema_version}_{apishortname}_{apiversion}.json


Is this comment still relevant, or is it cruft?

Yep still relevant, there's a lot of fields that are not yet filled out.

software-dov · 2022-01-13T22:15:28Z

gapic/generator/generator.py

@@ -324,8 +325,8 @@ def _get_file(
        template_name: str,
        *,
        opts: Options,
-        api_schema=api.API,
-        **context: Mapping,
+        api_schema: api.API,


Doh! Good catch.

software-dov · 2022-01-13T22:17:15Z

gapic/samplegen/samplegen.py

                    raise types.InvalidConfig(
                        "Sample config is invalid", valid)


Outside the scope of the review, but taking a second look, this isn't very helpful. Do you think it would be better to include the config or the filepath tothe config here?

Modified the error message to look like this:

if not valid: raise types.InvalidConfig( "Sample config in '{}' is invalid\n\n{}".format(config_fpath, cfg), valid)

software-dov · 2022-01-13T22:18:35Z

gapic/samplegen/samplegen.py

@@ -1062,7 +1055,7 @@ def generate_sample_specs(api_schema: api.API, *, opts) -> Generator[Dict[str, A
                yield spec


-def generate_sample(sample, api_schema, sample_template: jinja2.Template) -> str:
+def generate_sample(sample, api_schema, sample_template: jinja2.Template) -> Tuple[str, Any]:


Why use Any as the type of the second second element instead of the real type? Is it related to the # type: ignore a few lines down?

Yes, since the type is ignored I cannot use it in an annotation. I learned about mypy-protobuf from a PR with Tres though, so I'll open a separate PR to start type checking all the protobuf types.

software-dov · 2022-01-13T22:22:03Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2

@@ -207,6 +207,13 @@ class {{ service.async_client_name }}:
            {% endif %}
        r"""{{ method.meta.doc|rst(width=72, indent=8) }}

+        .. code-block::


Should the .. code-block:: be inside the if statement a few lines below?

Ahh good catch, this would have created a lot of noise in the regen PRs.

software-dov · 2022-01-13T22:22:49Z

tests/integration/goldens/asset/google/cloud/asset_v1/services/asset_service/async_client.py

@@ -210,6 +210,31 @@ async def export_assets(self,
        the export operation result. For regular-size resource parent,
        the export operation usually finishes within 5 minutes.

+
+        .. code-block::
+        from google.cloud import asset_v1


This indentation is a little unfortunate, but we can probably ignore it.

Hrrm I didn't notice this earlier, I'll try poking at the template to see where this is coming from.

amanda-tarafa

I left a couple of comments on the generated metadata file, but it looks good!

amanda-tarafa · 2022-01-14T10:12:45Z

tests/integration/goldens/asset/samples/generated_samples/snippet_metadata_v1_asset_v1.json

@@ -0,0 +1,1048 @@
+{


For the time beeing, we dropped versioning of the metadata scheam so you can drop the first v1 here.
Also, we are doing, where possible, snippet_metadata_{protopackage}.json, although the real important part is snippet_metadata*.json as the rest of the name is not going to be used to identify anything automatically.

amanda-tarafa · 2022-01-14T10:18:58Z

tests/integration/goldens/asset/samples/generated_samples/snippet_metadata_v1_asset_v1.json

+      "clientMethod": {
+        "async": true,
+        "method": {
+          "fullName": "AnalyzeIamPolicyLongrunning",


fullName shuld be the RPC method name qualified by the proto package and the service name
This looks more like shortName.

Ack, I'll switch this out for shortName.

amanda-tarafa · 2022-01-14T10:19:37Z

tests/integration/goldens/asset/samples/generated_samples/snippet_metadata_v1_asset_v1.json

+        "method": {
+          "fullName": "AnalyzeIamPolicyLongrunning",
+          "service": {
+            "shortName": "AssetService"


And fullName would be the service name qualified by the proto package.
Fine if this is one the ones you are adding later.

amanda-tarafa · 2022-01-14T10:20:27Z

tests/integration/goldens/asset/samples/generated_samples/snippet_metadata_v1_asset_v1.json

+          }
+        }
+      },
+      "file": "samples/generated_samples/cloudasset_generated_asset_v1_asset_service_analyze_iam_policy_longrunning_async.py",


This path should be relative to the location of this metadata file. Just double checking that's the case.

Ah it's currently relative to the library root. I'll remove the samples/generated_samples since the metadata sits in the same directory as the snippets.

gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2

busunkim96 and others added 17 commits January 4, 2022 22:57

Merge branch 'master' into snippets-in-docstrings

d356b12

chore: delete extra proto file

89df318

chore: fix generator.py

b50dd72

test: fix generator tests, format

7465546

Merge branch 'master' into snippets-in-docstrings

237f076

fix: adjust type annotation for _generate_samples_and_manifest

8e07b88

Merge branch 'snippets-in-docstrings' of github.com:googleapis/gapic-…

872c156

…generator-python into snippets-in-docstrings t push

chore: update goldens

e176b02

fix: stabilize metadata json

8315803

chore: regen metadata files

c58c5ae

fix: use OrderedDict

906be9c

fix:sort snippets by region tag

767a0ae

Merge branch 'master' into snippets-in-docstrings

2d1e738

chore: format

823ad2f

Merge branch 'snippets-in-docstrings' of github.com:googleapis/gapic-…

0ada00e

…generator-python into snippets-in-docstrings

fix: remove unnecessary conversion to dict

1e3defc

busunkim96 marked this pull request as ready for review January 13, 2022 18:08

busunkim96 requested review from a team as code owners January 13, 2022 18:08

busunkim96 requested review from software-dov and amanda-tarafa January 13, 2022 18:08

software-dov approved these changes Jan 13, 2022

View reviewed changes

amanda-tarafa approved these changes Jan 14, 2022

View reviewed changes

busunkim96 added 2 commits January 14, 2022 22:04

chore: remove unused imports

11d84b4

fix: fix left alignment issue in docstrings

446591a

software-dov reviewed Jan 18, 2022

View reviewed changes

gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 Outdated Show resolved Hide resolved

busunkim96 added 3 commits January 18, 2022 16:50

fix: fix file path in snippet metadata

f710bb7

fix: use method shortName field

d6f2490

Merge branch 'master' into snippets-in-docstrings

e358e25

busunkim96 added 2 commits January 18, 2022 17:31

chore: update goldens

02f1258

chore: fix code-block syntax

c7a41c5

busunkim96 merged commit 9e46031 into master Jan 18, 2022

busunkim96 deleted the snippets-in-docstrings branch January 18, 2022 18:39

This was referenced Jan 18, 2022

chore(master): release reverted #1133

Merged

chore(master): release 0.40.0 #1138

Closed

chore(master): release 0.60.0 #1139

Merged

chore(master): release 0.39.1 #1146

Closed

This was referenced Jan 26, 2022

chore(main): release 0.39.1 #1160

Closed

chore(main): release 0.39.1 #1162

Closed

chore(main): release 0.61.0 #1163

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: generate snippet metadata #1129

feat: generate snippet metadata #1129

busunkim96 commented Jan 11, 2022 •

edited

software-dov left a comment

software-dov Jan 13, 2022

software-dov Jan 13, 2022

busunkim96 Jan 14, 2022

software-dov Jan 13, 2022

busunkim96 Jan 14, 2022

software-dov Jan 13, 2022

software-dov Jan 13, 2022

busunkim96 Jan 14, 2022

software-dov Jan 13, 2022

busunkim96 Jan 14, 2022 •

edited

software-dov Jan 13, 2022

busunkim96 Jan 14, 2022

software-dov Jan 13, 2022

busunkim96 Jan 14, 2022

amanda-tarafa left a comment

amanda-tarafa Jan 14, 2022

amanda-tarafa Jan 14, 2022

busunkim96 Jan 15, 2022

amanda-tarafa Jan 14, 2022

amanda-tarafa Jan 14, 2022

busunkim96 Jan 15, 2022

		# NOTE(busunkim): Not all fields are yet populated in the snippet metadata.
		# Expected filename: snippet_metadata_{metadata_schema_version}_{apishortname}_{apiversion}.json

		raise types.InvalidConfig(
		"Sample config is invalid", valid)

feat: generate snippet metadata #1129

feat: generate snippet metadata #1129

Conversation

busunkim96 commented Jan 11, 2022 • edited

software-dov 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

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

busunkim96 Jan 14, 2022 • edited

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

amanda-tarafa 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

busunkim96 commented Jan 11, 2022 •

edited

busunkim96 Jan 14, 2022 •

edited