Decouple MetricsManager from AppNode #6557
Conversation
…ary public functions from the MetricsManager API
tconkling
added
the
security-assessment-completed
tconkling
changed the title
| // Update Metrics | ||
| this.metricsMgr.incrementDeltaCounter(getRootContainerName(deltaPath)) | ||
|
|
There was a problem hiding this comment.
All the removed logic from this function is now in SegmentMetricsManager.handleDeltaMessage
frontend/src/App.test.tsx
Outdated
| PageInfo, | ||
| PageNotFound, | ||
| PagesChanged, | ||
| Delta, |
There was a problem hiding this comment.
supernit: Should this go to line 26 so it's alphabetical 😆
There was a problem hiding this comment.
Looking at other files though, it doesn't seem like we retain that so I don't think it's necessary. Just something I noticed... 😅
There was a problem hiding this comment.
Yeah - this is the sort of thing that we should enforce with linters if we care, because it's purely stylistic.
| this.send(evName, evData) | ||
| } | ||
|
|
||
| public handleDeltaMessage(delta: Delta, metadata: ForwardMsgMetadata): void { |
There was a problem hiding this comment.
nit: I believe the delta stats & custom component event (which requires the delta counter) is actually unused and planned to be removed by the data team. So, maybe we can already remove it right now. Probably something to sync with @blackary.
There was a problem hiding this comment.
Nice. I'm going to merge this as-is (because it unblocks the StreamlitLib project), and then work on a followup after chatting with @blackary.
* develop: Decouple MetricsManager from AppNode (streamlit#6557) Fix top padding on sidebar when embed is true (streamlit#6565) Add support for cell and column header tooltips in the dataframe component (streamlit#6561) Update dataframe column properties on frontend (streamlit#6554) Show warning for unsafe integer cells in `st.dataframe` (streamlit#6549) Add icon for editable columns in `st.data_editor` (streamlit#6550) Unify missing values to None in the returned datastructure by `st.data_editor`. (streamlit#6544) Clean up and reorganize element tree module (streamlit#6522) ESLint: use `--cache` flag (30x speedup!) (streamlit#6555) Replace `st.connection` with `st.experimental_connection` in docstring examples (streamlit#6553) Improve editing on touch devices for `st.data_editor` (streamlit#6548) Move pandas styler logic to dedicated module (streamlit#6543)
3 participants
AppNodecurrently depends onMetricsManager. If we release the upcoming "StreamlitLib" project as is, StreamlitLib users will have to supply their own MetricsManager implementation. With this PR,MetricsManagerno longer needs to be part of StreamlitLib.AppNodeno longer knows aboutMetricsManagerApp.handleDeltaMessagesends Deltas toMetricsManagerMetricsManagerpublic APISegmentMetricsManagertests