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

fix: add fieldMask for getDevice and listDevices #64

Merged
merged 3 commits into from Feb 5, 2021
Merged

Conversation

gcseh
Copy link
Contributor

@gcseh gcseh commented Feb 4, 2021

Fixes #178

@gcseh gcseh requested review from telpirion and munkhuushmgl Feb 4, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 4, 2021
@product-auto-label product-auto-label bot added the api: cloudiot Issues related to the googleapis/python-iot API. label Feb 4, 2021
@gcseh gcseh added the samples Issues that are directly related to samples. label Feb 4, 2021
@gcseh gcseh changed the title Add fieldMask for getDevice and listDevices fix: add fieldMask for getDevice and listDevices Feb 4, 2021
Copy link

@telpirion telpirion left a comment

Two small comments, but please do consider addressing them.

We typically use black to format our code.

device = client.get_device(request={"name": device_path})
# See full list of device fields: https://cloud.google.com/iot/docs/reference/cloudiot/rest/v1/projects.locations.registries.devices
# Warning! Use snake_case field names.
fieldMask = gp_field_mask.FieldMask(paths=['id', 'name', 'num_id', 'credentials', 'last_heartbeat_time', 'last_event_time', 'last_state_time', 'last_config_ack_time', 'last_config_send_time', 'blocked', 'last_error_time', 'last_error_status', 'config', 'state', 'log_level', 'metadata', 'gateway_config'])

Choose a reason for hiding this comment

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

I suggest breaking this up into separate lines to avoid linter issues.

devices = list(client.list_devices(request={"parent": registry_path}))
# See full list of device fields: https://cloud.google.com/iot/docs/reference/cloudiot/rest/v1/projects.locations.registries.devices
# Warning! Use snake_case field names.
fieldMask = gp_field_mask.FieldMask(paths=['id', 'name', 'num_id', 'credentials', 'last_heartbeat_time', 'last_event_time', 'last_state_time', 'last_config_ack_time', 'last_config_send_time', 'blocked', 'last_error_time', 'last_error_status', 'config', 'state', 'log_level', 'metadata', 'gateway_config'])

Choose a reason for hiding this comment

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

Same as above. It also improves readability (and by extension, maintainability).

@gcseh gcseh marked this pull request as ready for review Feb 5, 2021
@gcseh gcseh requested review from a team as code owners Feb 5, 2021
@gcseh gcseh requested a review from busunkim96 Feb 5, 2021
@gcseh gcseh merged commit 24c9c9a into master Feb 5, 2021
9 checks passed
@gcseh gcseh deleted the add-field-mask branch Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: cloudiot Issues related to the googleapis/python-iot API. cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve FieldMask documentation
3 participants