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

Allowing for arbitrarily nested dictionaries in _log_entry_mapping_pb. #2589

Merged
merged 2 commits into from
Oct 24, 2016

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Oct 22, 2016

Fixes #2552.

@waprin Do you think it's worthwhile to add a system test for this / update an existing system test with a nested dictionary?

@dhermes dhermes added the api: logging Issues related to the Cloud Logging API. label Oct 22, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 22, 2016
# NOTE: ``json.dumps`` is wasteful here because internally,
# ``Parse`` will just call ``json.loads``. However,
# there is no equivalent public function to parse on raw
# dictionaries, so we waste cycles on parse/unparse.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@daspecster
Copy link
Contributor

@dhermes, did you try this on the service and confirm that the data gets logged correctly?
I haven't found an issue but I'm curious if the upstream service captures the nested data properly.

@dhermes
Copy link
Contributor Author

dhermes commented Oct 24, 2016

@daspecster

>>> from google.cloud import logging
>>> client = logging.Client()
>>> logger = client.logger('errors')
>>> 
>>> payload = {
...     'a': {
...         'b': 'c',
...     }
... }
>>> logger.log_struct(payload)
>>> 

@waprin
Copy link
Contributor

waprin commented Oct 24, 2016

@dhermes thanks for figuring this out for me, works for me. As for system test, I think it'd be trivial to add it just by adding one level of nesting to the current log_struct system test.

@dhermes
Copy link
Contributor Author

dhermes commented Oct 24, 2016

@waprin Good find on ParseDict, updated it in the 2nd commit (and also updated the system test).

@waprin
Copy link
Contributor

waprin commented Oct 24, 2016

LGTM!

@dhermes dhermes merged commit 05b5059 into googleapis:master Oct 24, 2016
@dhermes dhermes deleted the fix-2552 branch October 24, 2016 18:19
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
Allowing for arbitrarily nested dictionaries in _log_entry_mapping_pb.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: logging Issues related to the Cloud Logging API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants