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

Table-setting for generating protos. #35

Merged
merged 2 commits into from
Sep 26, 2018
Merged

Table-setting for generating protos. #35

merged 2 commits into from
Sep 26, 2018

Conversation

lukesneeringer
Copy link
Contributor

@lukesneeringer lukesneeringer commented Sep 22, 2018

This commit makes several schema edits and additions in order to facilitate generating pb2 replacements directly.

In particular, it:

  • Adds support for a $proto rewrite for filenames (such files receive a proto variable), and adds loader support for such.
  • Refactors Address to include the name of the object it is attached to, enabling it to be used as the ident object.
  • Refactors the various *_ident properties into the address, and re-orders template use (e.g. sphinx_ident is now ident.sphinx).
    • Additionally, python_ident is now just ident in templates.
  • Adds a rel method for getting identifiers relative to the file being generated. This deals with a problem that will surface in replacing pb2s where a file would attempt to import or reference itself.

LROs are slightly broken in a slightly different way than before (note that they were broken in the status quo ante due to a bug in OperationType), and OperationType turned out to be a mistake. Filed #34 to track this, and it will be fixed after pb2 replacements are introduced (since they will significantly alter what the fix is).

This commit makes several schema edits and additions in order to facilitate
generating pb2 replacements directly.

In particular, it:

  * Adds support for a `$proto` rewrite for filenames (such files
    receive a `proto` variable), and adds loader support for such.
  * Refactors `Address` to include the `name` of the object it is
    attached to, enabling it to be used as the `ident` object.
  * Refactors the various `*_ident` properties into the address,
    and re-orders template use (e.g. `sphinx_ident` is now
    `ident.sphinx`).
      * Additionally, `python_ident` is now just `ident` in templates.
  * Adds a `rel` method for getting identifiers relative to the
    file being generated. This deals with a problem that will surface
    in replacing pb2s where a file would attempt to import or reference
    itself.

LROs are slightly broken in a slightly different way than before
(note that they were broken in the status quo ante due to a bug
in `OperationType`), and `OperationType` turned out to be a mistake.
Filed #34 to track this, and it will be fixed after pb2 replacements
are introduced (since they will significantly alter what the fix is).
@codecov
Copy link

codecov bot commented Sep 22, 2018

Codecov Report

Merging #35 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #35   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          15     15           
  Lines         522    552   +30     
  Branches       91    101   +10     
=====================================
+ Hits          522    552   +30
Impacted Files Coverage Δ
gapic/utils/lines.py 100% <ø> (ø) ⬆️
gapic/generator/generator.py 100% <100%> (ø) ⬆️
gapic/schema/wrappers.py 100% <100%> (ø) ⬆️
gapic/schema/api.py 100% <100%> (ø) ⬆️
gapic/schema/metadata.py 100% <100%> (ø) ⬆️
gapic/generator/loader.py 100% <100%> (ø) ⬆️

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 e77a870...d61e844. Read the comment docs.

lukesneeringer pushed a commit that referenced this pull request Sep 22, 2018
Copy link

@landrito landrito left a comment

Choose a reason for hiding this comment

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

LGTM with nits.

"""Return the appropriate module name for this service.

Returns:
str: The service name, in snake case.

This comment was marked as spam.

This comment was marked as spam.


@cached_property
def top(self) -> 'Proto':
"""Return a proto shim which is only aware of top-level objects."""

This comment was marked as spam.

This comment was marked as spam.

@@ -282,11 +305,17 @@ def _load_children(self, children: Sequence, loader: Callable, *,
path (Tuple[int]): The location path up to this point. This is
used to correspond to documentation in
``SourceCodeInfo.Location`` in ``descriptor.proto``.

Return:
Mapping: A sequence of the objects that were loaded.

This comment was marked as spam.

This comment was marked as spam.

gapic/schema/wrappers.py Show resolved Hide resolved
)

def rel(self, address: 'Address') -> str:

This comment was marked as spam.

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants