Skip to content

Update logging codegen - #1629

Closed
geigerj wants to merge 3 commits into
googleapis:grpc-logging-featurefrom
geigerj:grpc-logging-feature
Closed

Update logging codegen#1629
geigerj wants to merge 3 commits into
googleapis:grpc-logging-featurefrom
geigerj:grpc-logging-feature

Conversation

@geigerj

@geigerjgeigerj commented Mar 17, 2016

Copy link
Copy Markdown
Contributor

Reflects updates to the code generation made between January and now.

An sample usage snippet is available here: https://gist.github.com/geigerj/de050b53f8887e9d7a07

@tbetbetbe
@anthmgoogle
@jgeewax

@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 17, 2016
@tbetbetbe

tbetbetbe commented Mar 17, 2016

Copy link
Copy Markdown

@jcanizales

dhermes
dhermes reviewed Mar 18, 2016
View reviewed changes
Comment threadgcloud/logging/config_service_v2_api.py
# EDITING INSTRUCTIONS
# This file was generated from the file
# https://github.com/google/googleapis/blob/7710ead495227e80a0f06ceb66bdf3238d926f77/google/logging/v2/logging_config.proto,
# https://github.com/google/googleapis/blob/master/google/logging/v2/logging_config.proto,

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tseaver

tseaver commented Mar 18, 2016

Copy link
Copy Markdown
Contributor

@geigerj thanks for the new PR! I'll skip the detail-oriented review since @dhermes is tackling that bit, but just wanted to let you know that looking at the generated wrappers for the protobuf/gRPC API is quite illuminating, as I'm working on the REST/JSON wrappers this month.

This was referenced Mar 18, 2016
Open
Closed
dhermes
dhermes reviewed Mar 18, 2016
View reviewed changes
Comment threadgcloud/logging/config_service_v2_api.py Outdated
if app_name is None:
app_name = 'gax'
if app_version is None:
app_version = google.gax.__version__

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes

dhermes commented Mar 18, 2016

Copy link
Copy Markdown
Contributor

Design question. If the code is generated, couldn't you use the contents of the YAML file to just populate variables in the module? It seems that having to bear the weight of file I/O and parsing YAML is unneeded.

dhermes
dhermes reviewed Mar 18, 2016
View reviewed changes
Comment threadgcloud/logging/logging_service_v2_api.py
'https://www.googleapis.com/auth/logging.admin',
'https://www.googleapis.com/auth/logging.read',
'https://www.googleapis.com/auth/cloud-platform.read-only',
'https://www.googleapis.com/auth/cloud-platform', )

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tbetbetbetbetbetbe mentioned this pull request Mar 18, 2016
Closed
dhermes
dhermes reviewed Mar 18, 2016
View reviewed changes
Comment threadgcloud/logging/logging_service_v2_api.py Outdated
google_apis_agent = '{}-{}/{}/gax-{}/{}'.format(
app_name, app_version, self._CODE_GEN_NAME_VERSION,
google.gax.__version__,
'python-{}'.format(platform.python_version()))

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes

dhermes commented Mar 18, 2016

Copy link
Copy Markdown
Contributor

OK I finished my comments.

@tbetbetbetbetbetbe mentioned this pull request Mar 18, 2016
Closed
@theacodes

theacodes commented Mar 18, 2016

Copy link
Copy Markdown
Contributor

Design question. If the code is generated, couldn't you use the contents of the YAML file to just populate variables in the module? It seems that having to bear the weight of file I/O and parsing YAML is unneeded.

I feel the same way. There seems to be no real benefit to doing it this way.

@tbetbetbe

tbetbetbe commented Mar 18, 2016

Copy link
Copy Markdown

Design question. If the code is generated, couldn't you use the contents of the YAML file to just populate variables in the module? It seems that having to bear the weight of file I/O and parsing YAML is unneeded.
I feel the same way. There seems to be no real benefit to doing it this way.

In the not too far-off future, the plan is to allow user configuration of api settings via yaml

  • the exact mechanism is still under discussion
  • hopefully, this will look a bit like application default creds, e.g,
    • there is a yaml file in the package will contain fallback config values
    • users will be specify an override either by specifying an environment variable or by copying to the settings to a well-known location

This was referenced Mar 18, 2016
Closed
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

cla: yesThis human has signed the Contributor License Agreement.do not mergeIndicates a pull request not ready for merge, due to either quality or timing.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@geigerj@tbetbetbe@tseaver@dhermes@theacodes@broady@jgeewax@googlebot