chore(cubesql): Support extra_id for Session #8612
Conversation
chore(cubesql): Allow to do custom representations for sessions
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 8 Skipped Deployments
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8612 +/- ##
==========================================
- Coverage 82.53% 82.51% -0.02%
==========================================
Files 209 209
Lines 76688 76711 +23
==========================================
+ Hits 63292 63297 +5
- Misses 13396 13414 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -62,6 +62,8 @@ pub enum QueryState { | |||
| pub struct SessionState { | |||
| // connection id, immutable | |||
| pub connection_id: u32, | |||
| // Can be UUID or anything else. MDX uses UUID | |||
| pub extra_id: Option<String>, | |||
There was a problem hiding this comment.
Do we want to limit extra_id to UTF-8? Maybe Vec<u8>/Bytes would be better. UUID is representable with 16 raw bytes, AFAIK uuid crate does exactly that.
| @@ -447,17 +412,58 @@ pub struct SessionProcessList { | |||
| pub database: Option<String>, | |||
| } | |||
|
|
|||
| impl From<&Arc<Session>> for SessionProcessList { | |||
There was a problem hiding this comment.
Do we need Arc here? It feels like impl From<&Session> should be enough here
| } | ||
|
|
||
| pub async fn process_list(self: &Arc<Self>) -> Vec<SessionProcessList> { | ||
| pub async fn map_sessions<T: for<'a> From<&'a Arc<Session>>>(self: &Arc<Self>) -> Vec<T> { |
There was a problem hiding this comment.
Current map_sessions usages don't need Arc part, From<&'a Session> should be enough.
Arc here can be useful (e.g. to be able to clone Arc instance), but as it is now it's bit confusing to me
|
|
||
| let mut guard = self.sessions.write().await; | ||
|
|
||
| guard.insert(connection_id, session_ref.clone()); | ||
| if let Some(extra_id) = extra_id { | ||
| if guard.uid_to_session.contains_key(&extra_id) { |
There was a problem hiding this comment.
AFAIU, extra_id is user-controlled, so this place can trigger hashing of strings of arbitrary length.
Do we want to limit it?
No description provided.