solara icon indicating copy to clipboard operation
solara copied to clipboard

feat: improve logging of reactive variable changes

Open iisakkirotko opened this issue 1 year ago • 7 comments

The way that the log_level-settings is set is kind of hacky right now, but we need to discuss implementing a real solution in a different way

iisakkirotko avatar Feb 22 '24 14:02 iisakkirotko

  • #517 Graphite 👈
  • master

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @iisakkirotko and the rest of your teammates on Graphite Graphite

iisakkirotko avatar Feb 22 '24 14:02 iisakkirotko

So testing the performance, the cost is only at start-up of the server, but with this change it takes about 4s longer to start the server than without. I'll take a look at what part of the change requires so much time next week.

P.S. It seems like reactive variables get initialized twice?

iisakkirotko avatar Feb 23 '24 09:02 iisakkirotko

This PR led to a discussion about logging, for that I opened an issue https://github.com/widgetti/solara/issues/527

maartenbreddels avatar Feb 28 '24 08:02 maartenbreddels

Btw, I expected this PR to build on top of #515 ? Maybe a squash issue?

TODO:

  • [x] find out why it takes so much more time (Iisakki)
  • [ ] find out why it seems to initialize twice (Maarten)

maartenbreddels avatar Feb 28 '24 08:02 maartenbreddels

Btw, I expected this PR to build on top of #515 ? Maybe a squash issue?

To me these changes feel unrelated (even though they both use inspect.stack()), since one is about logging, while the other is fixing an issue. You could also argue that #515 could be based on this one, and make use of var._varname.

iisakkirotko avatar Mar 01 '24 14:03 iisakkirotko

The performance of various ways to get caller names was tested here, from which it's pretty fair to say that we chose the slowest way to do this initially. I'll take a look at using a rewriting this PR using one of the better performing methods.

iisakkirotko avatar Mar 05 '24 15:03 iisakkirotko

Your Render PR Server URL is https://solara-stable-pr-517.onrender.com.

Follow its progress at https://dashboard.render.com/web/srv-co18g18l6cac73ftelk0.

render[bot] avatar Mar 26 '24 08:03 render[bot]