Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add gapic metadata file #781

Merged
merged 21 commits into from
Mar 2, 2021

Conversation

software-dov
Copy link
Contributor

The GAPIC metadata file is used to track code, samples, and test
coverage for every RPC and library method.

The GAPIC metadata file is used to track code, samples, and test
coverage for every RPC and library method.
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 17, 2021
@software-dov
Copy link
Contributor Author

Note: hand testing of the generated file indicates that it is valid JSON and behaves in a reasonable manner.

@codecov
Copy link

codecov bot commented Feb 17, 2021

Codecov Report

Merging #781 (70b79cf) into master (7c185e8) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #781   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        27    +1     
  Lines         1608      1645   +37     
  Branches       328       336    +8     
=========================================
+ Hits          1608      1645   +37     
Impacted Files Coverage Δ
gapic/utils/case.py 100.00% <ø> (ø)
gapic/utils/reserved_names.py 100.00% <ø> (ø)
gapic/generator/generator.py 100.00% <100.00%> (ø)
gapic/schema/api.py 100.00% <100.00%> (ø)
gapic/schema/wrappers.py 100.00% <100.00%> (ø)
gapic/utils/__init__.py 100.00% <100.00%> (ø)
gapic/utils/checks.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b199b14...70b79cf. Read the comment docs.

Copy link
Contributor

@vchudnov-g vchudnov-g left a comment

Choose a reason for hiding this comment

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

Looks good, but I just have to ask: is it not practical to populate a metadata proto object with the data and then serialize it to JSON? That way we're more tightly tied to the source of truth (the .proto) and we don't have to worry about presentation logic ourselves. I want to avoid shutting the .proto out of the loop, as it's conceivable we may want to add items later. (That said, if there's a practical reason for emitting straight JSON, that's fine, but let's mention it in the PR.)

@software-dov
Copy link
Contributor Author

There is no deep reason for emitting straight JSON.
This is a reading comprehension failure on my part; I simply missed the bottom part of the design doc that described where the corresponding proto file lived and that we should use it. I'll amend the PR immediately.

@software-dov
Copy link
Contributor Author

NOTE: in light of a design doc reread, this PR is on hold until https://pypi.org/project/googleapis-common-protos/ is updated to include the python binding for metadata.proto.

@vchudnov-g
Copy link
Contributor

Any updates or blockers on the the googleapis-common-protos issue?

@busunkim96
Copy link
Contributor

@software-dov Opened googleapis/python-api-common-protos#38. If you'd like to try a pip VCS install git+https://github.com/googleapis/python-api-common-protos.git@add-gapic-metadata should work

@software-dov
Copy link
Contributor Author

Approved the change to api-common-protos and just about to experiment with the it in local code.

The GAPIC metadata file is used to track code, samples, and test
coverage for every RPC and library method.
@software-dov
Copy link
Contributor Author

The updated api-common-protos seems to be fine.

@software-dov
Copy link
Contributor Author

This is a severely truncated sample from Bigtable Admin:

{
  "comment": "This file maps proto services/RPCs to the corresponding library clients/methods",
  "language": "python",
  "libraryPackage": "google.bigtable.admin_v2",
  "protoPackage": "google.bigtable.admin.v2",
  "schema": "1.0",
  "services": {
    "BigtableTableAdmin": {
      "clients": {
        "grpc": {
          "libraryClient": "BigtableTableAdminClient",
          "rpcs": {
            "CheckConsistency": {
              "methods": [
                "check_consistency"
              ]
            },
            "CreateBackup": {
              "methods": [
                "create_backup"
              ]
            },
            "CreateTable": {
              "methods": [
                "create_table"
              ]
            },
          }
        },
        "grpcAsync": {
          "libraryClient": "BigtableTableAdminAsyncClient",
          "rpcs": {
            "CheckConsistency": {
              "methods": [
                "check_consistency"
              ]
            },
            "CreateBackup": {
              "methods": [
                "create_backup"
              ]
            },
            "CreateTable": {
              "methods": [
                "create_table"
              ]
            },
          }
        }
      }
    }
  }
}

@software-dov software-dov requested a review from vchudnov-g March 2, 2021 19:46
@software-dov software-dov merged commit 5dd8fcc into googleapis:master Mar 2, 2021
@software-dov software-dov deleted the gapic-metadata branch March 2, 2021 22:59
Copy link
Contributor

@vchudnov-g vchudnov-g left a comment

Choose a reason for hiding this comment

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

Looks good. One minor suggestion.
(Apologies for not getting to this earlier.)

tests/unit/schema/test_api.py Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants