feat: set_log_filter rpc and start_time rpc#410
feat: set_log_filter rpc and start_time rpc#410MicaiahReid merged 7 commits intosolana-foundation:mainfrom
Conversation
MicaiahReid
left a comment
There was a problem hiding this comment.
Hey @JkrishnaD, thanks for the PR!
I've got a few notes:
Remove set_log_filter method
Sadly, I think we should remove the set_log_filter admin method. I don't see it as particularly valuable to the user, and our log management is a bit more complicated than you have here. We manage logs for the CLI and a written log, so we have a designated configuration on startup of the simnet. I think what we have at the CLI level is sufficient, so we can just remove this admin RPC method completely. (both from the implementation and the AdminRpc trait.
Fixes for start_time
The current implementation of the start_time is gated behind the subgraph feature, which is unnecessary. Actually, it doesn't need to be using the PluginManagerCommand at all. The use of an rx/tx pair to send a command to get a time back is necessary.
I think a better approach is:
- Add a
start_timefield to theSurfnetSvmstruct in thesvm.rsfile - Add a function to the
SurfnetSvmLockerstructure (inlocker.rs) to get this value - Have something like the following in the
start_timeadmin RPC function to just directly fetch the time:
let svm_locker = meta.get_svm_locker()?;
let start_time = svm_locker.get_start_time();
Ok(start_time)
@MicaiahReid I’ve applied all the requested changes:
Please take another look when you get a chance. Thanks for the clear guidance! |
MicaiahReid
left a comment
There was a problem hiding this comment.
This is great, thanks for the quick turnaround! Just two small comments - some artifacts of the previous implementation that weren't removed, then we'll be good to merge!
|
@MicaiahReid ser check the implementation now |
MicaiahReid
left a comment
There was a problem hiding this comment.
Looks good, thank you so much @JkrishnaD!!
Changes
set_log_filteradmin RPC for updating the system log level.start_timeadmin RPC to return the runloop start timestamp.PluginManagerCommandvariants.Issue: #330