Uh oh!
There was an error while loading. Please reload this page.
fix: allow reading logs from non-project paths - #444
Conversation
| def logger_name_from_path(path): | ||
| def logger_name_from_path(path, project=None): |
There was a problem hiding this comment.
nit: Perhaps I am not well familiar with Python, but is there a way to create another logger_name_from_path function with project parameter? I think that it would be better then having parameters with default values
There was a problem hiding this comment.
I think this is the Pythonic way to do it. Especially since this function is just directly calling another function. If it wasn't a public function already in use, I would prefer to remove this one altogether
What's the argument against default parameters?
| logger_name = logger_name_from_path(logger_fullname, client.project) | ||
| logger = loggers[logger_fullname] = client.logger(logger_name) | ||
| except ValueError: | ||
| # log name is not scoped to a project. Leave logger as None |
There was a problem hiding this comment.
Is this comment true? Seems that logger is initialized with value returned from loggers.get(logger_fullname) call...
There was a problem hiding this comment.
loggers.get(logger_fullname) will return None if the loggers dict doesn't contain logger_fullname
If it's None. Lines 171 to 173 will try to contruct a new logger associated with logger_name, which will work if logger_name is a project id. If it's an organization or a folder, it will throw an exception. In that case, we can just leave the logger as empty and move on
Previously, the entry-parsing code assumed all logs originated from a project, but the list_logs API allows reading from folders, organizations, and billingIds as well. If a user attempted to read from one of these non-project paths, the library would fail to build the LogEntry object and crash. This PR fixes that issue, and is more forgiving in its entry parsing logic.
Now, if a log is found with a parent path that does not represent the active project, the
entry.loggerfield will beNone.Fixes#399