feat: get_ and tail_logs methods to Device #16
Conversation
| * @param end_time_ns End time in nanoseconds since epoch (optional) | ||
| * @return std::vector<std::string> containing log entries | ||
| */ | ||
| [[nodiscard]] auto get_logs( |
There was a problem hiding this comment.
For both methods, could you return a science::Status -- its our convention for reporting any errors (rather than throwing)
You could either return a tuple [Status, string], or, more c++-like, add arguments at the end of each function's signature for outputs, e.g.
get_logs(const std::string& log_level, ..., std::vector<std::string>* output)
| if (since_ms.has_value()) { | ||
| request.set_since_ms(since_ms.value()); | ||
| } else { | ||
| if (start_time_ns.has_value()) { |
There was a problem hiding this comment.
nice!
while we're here, mind checking that start_time_ns is less than end_time_ns, and if it's not, return an error science::status?
| logs.push_back(entry.message()); | ||
| } | ||
| } else { | ||
| std::cerr << "Error getting logs: " << status.error_message() << std::endl; |
There was a problem hiding this comment.
don't print an error (because consumers won't be able to turn these logs off) -- instead report this error message in a returned science::status (and preferably don't capitalize the first string so they're easier to chain)
| * @return std::vector<std::string> containing the most recent log entries | ||
| */ | ||
| [[nodiscard]] auto tail_logs( | ||
| const std::string& log_level = "INFO", |
There was a problem hiding this comment.
let's add a LogLevel enum or use the existing proto one rather than accepting raw strings.
I like the default value!
| return logs; | ||
| } | ||
|
|
||
| auto Device::tail_logs( |
There was a problem hiding this comment.
I'm realizing that the gRPC client in this device class is sync, when we probably want async (whoops). Ideally a consumer could keep a 'tail_logs' call open & running while they make other device calls, but with this current implementation that's not possible. I'll add a backlog task to change the client from sync to async -- for now, lets not include this
add boilerplate methods to device class similar to synapse-python