| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
q: why does the example you shared in #121 (comment) emit a textPayload not a jsonPayload as I'd expect when you give the log a structured data like a dictionary? |
Sorry, something went wrong.
|
@taeold The code was adapted from the node logger |
Sorry, something went wrong.
|
@exaby73 I'm not sure I understand what you mean. The purpose of nodejs logger is so that it's easy to emit structured log entry from valid JS object and not to emit them as just text message. e.g. exports.helloLogs = functions.https.onRequest(async (request, response) => {
functions.logger.warn("Hello wolrd!", { foo: "bar" });
response.send("hello world");
});
If the python logger isn't doing any work to generate structued logs using jsonPayload and can't set appropriate logging levels that corresponds to logging levels in Cloud Logging, then it's no more useful than print() calls. Can you confirm if:
|
Sorry, something went wrong.
|
@taeold I've updated the logger to support jsonPayload. The *args will be joined with a space to produce the message key and the **kwargs will be spread into the jsonPayload. Here is the result of calling logger.log("Hello World!", foo="Bar"):
|
Sorry, something went wrong.
| uid=token_data["uid"], | ||
| email=token_data.get("email"), | ||
| email_verified=token_data.get("email_verified"), | ||
| email_verified=bool(token_data.get("email_verified")), |
There was a problem hiding this comment.
hmm this feels unrelated? should we make a separate PR for this or is this necessary for log impl?
Sorry, something went wrong.
There was a problem hiding this comment.
mypy was updated and was causing CI to fail because the types didn't match. I'm open to suggestions. This is mainly a linting fix
Sorry, something went wrong.
There was a problem hiding this comment.
maybe clearest to make the mypy related linting fixes in a separate PR, and have only the logging things here.
Sorry, something went wrong.
There was a problem hiding this comment.
Without these, the CI doesn't pass. Possibly can make a separate PR for these that can merged into main, before we merge this in so that we can have those changes rebased into this branch. I'll look into doing this next week :)
Sorry, something went wrong.
There was a problem hiding this comment.
eh let's save ourselves some time and push it in this PR.
Sorry, something went wrong.
| elif precision_timestamp == PrecisionTimestamp.SECONDS: | ||
| return second_timestamp_conversion(time) | ||
|
|
||
| raise ValueError("Invalid timestamp") |
There was a problem hiding this comment.
likewise - related to log impl?
Sorry, something went wrong.
There was a problem hiding this comment.
Again a lint failure here because the function wasn't ending in a return and the if, elif is not exhaustive
Sorry, something went wrong.
| uid=token_data["uid"], | ||
| email=token_data.get("email"), | ||
| email_verified=token_data.get("email_verified"), | ||
| email_verified=bool(token_data.get("email_verified")), |
There was a problem hiding this comment.
eh let's save ourselves some time and push it in this PR.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Closes #79