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.

Sign up for GitHub

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

Jump to bottom

regen iot with toolkit changes (WIP - do not merge) #5472

Closed
wants to merge 2 commits into from
Closed

regen iot with toolkit changes (WIP - do not merge) #5472

wants to merge 2 commits into from

Conversation

andreamlin
Copy link
Contributor

No description provided.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 12, 2018
@andreamlin andreamlin removed the request for review from lukesneeringer June 12, 2018 05:52
@andreamlin andreamlin mentioned this pull request Jun 12, 2018
Merged
@tseaver
Copy link
Contributor

tseaver commented Jun 12, 2018

The try: ... except AttributeError: pass pattern in the generated code is confusing, and seems often unnecessary (e.g., where the value is known to be a string). Where an object might not have the required value, this looks clearer to me:

header_value = getattr(target, 'header_value', None)
if header_value is not None:
    routing_header = google.api_core.gapic_v1.routing_header.to_grpc_metadata(
        [('header_name', header_value)], )
    metadata.append(routing_header)

tseaver
tseaver reviewed Jun 12, 2018
View reviewed changes
iot/google/cloud/iot_v1/gapic/device_manager_client.py Outdated
routing_header = google.api_core.gapic_v1.routing_header.to_grpc_metadata(
[('parent', parent)], )
metadata.append(routing_header)
routing_header = [('parent', parent)]
except AttributeError:
pass

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

iot/google/cloud/iot_v1/gapic/device_manager_client.py Outdated
routing_header = google.api_core.gapic_v1.routing_header.to_grpc_metadata(
[('name', name)], )
metadata.append(routing_header)
routing_header = [('name', name)]
except AttributeError:
pass

This comment was marked as spam.

Sign in to view

iot/google/cloud/iot_v1/gapic/device_manager_client.py Outdated
routing_header = google.api_core.gapic_v1.routing_header.to_grpc_metadata(
[('device_registry.name', device_registry.name)], )
metadata.append(routing_header)
routing_header = [('device_registry.name', device_registry.name)]
except AttributeError:
pass

This comment was marked as spam.

Sign in to view

iot/google/cloud/iot_v1/gapic/device_manager_client.py Outdated
routing_header = google.api_core.gapic_v1.routing_header.to_grpc_metadata(
[('name', name)], )
metadata.append(routing_header)
routing_header = [('name', name)]
except AttributeError:
pass

This comment was marked as spam.

Sign in to view

iot/google/cloud/iot_v1/gapic/device_manager_client.py Outdated
routing_header = google.api_core.gapic_v1.routing_header.to_grpc_metadata(
[('parent', parent)], )
metadata.append(routing_header)
routing_header = [('parent', parent)]
except AttributeError:
pass

This comment was marked as spam.

Sign in to view

iot/google/cloud/iot_v1/gapic/device_manager_client.py Outdated
routing_header = google.api_core.gapic_v1.routing_header.to_grpc_metadata(
[('name', name)], )
metadata.append(routing_header)
routing_header = [('name', name)]
except AttributeError:
pass

This comment was marked as spam.

Sign in to view

iot/google/cloud/iot_v1/gapic/device_manager_client.py Outdated
routing_header = google.api_core.gapic_v1.routing_header.to_grpc_metadata(
[('name', name)], )
metadata.append(routing_header)
routing_header = [('name', name)]
except AttributeError:
pass

This comment was marked as spam.

Sign in to view

iot/google/cloud/iot_v1/gapic/device_manager_client.py Outdated
routing_header = google.api_core.gapic_v1.routing_header.to_grpc_metadata(
[('resource', resource)], )
metadata.append(routing_header)
routing_header = [('resource', resource)]
except AttributeError:
pass

This comment was marked as spam.

Sign in to view

iot/google/cloud/iot_v1/gapic/device_manager_client.py Outdated
routing_header = google.api_core.gapic_v1.routing_header.to_grpc_metadata(
[('resource', resource)], )
metadata.append(routing_header)
routing_header = [('resource', resource)]
except AttributeError:
pass

This comment was marked as spam.

Sign in to view

iot/google/cloud/iot_v1/gapic/device_manager_client.py Outdated
routing_header = google.api_core.gapic_v1.routing_header.to_grpc_metadata(
[('resource', resource)], )
metadata.append(routing_header)
routing_header = [('resource', resource)]
except AttributeError:
pass

This comment was marked as spam.

Sign in to view

@andreamlin
Copy link
Contributor Author

andreamlin commented Jun 12, 2018 •
edited
Loading

Thanks for the review. I'll file another issue in https://github.com/googleapis/gapic-generator to fix the style comments.

edit: @tseaver Are any of your comments blockers?

@tseaver
Copy link
Contributor

tseaver commented Jun 12, 2018

@andreamlin You're welcome! The more significant issue is the case where there is an attribute to access, except the object might be a dict with the same key (at least per the argument specs for the enclosing functions).

@andreamlin andreamlin added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 13, 2018
@tseaver tseaver added the api: cloudiot Issues related to the IoT Core API. label Jun 26, 2018
@tseaver tseaver added the codegen label Aug 8, 2018
@tseaver tseaver closed this Aug 20, 2018
@andreamlin andreamlin mentioned this pull request Apr 9, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tseaver tseaver tseaver left review comments

Assignees
No one assigned
Labels
api: cloudiot Issues related to the IoT Core API. cla: yes This human has signed the Contributor License Agreement. codegen do not merge Indicates 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.

None yet
3 participants
@andreamlin @tseaver @googlebot