-
Notifications
You must be signed in to change notification settings - Fork 21
[DATASOURCE]: Persistent data handler update #478
base: main
Are you sure you want to change the base?
Conversation
715c00a
to
d677938
Compare
0511d22
to
770d8b5
Compare
ab10a4e
to
a70412a
Compare
bad20f1
to
30232ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am okay with merging this. @marsninja if you sign off, we can 🚢 this.
f611dbc
to
0e8e210
Compare
909c47c
to
a1f032b
Compare
e988be6
to
410bebb
Compare
CHANGES: - check if current root is super root - check if it's owned by the current root - check if target's root given access to current root - check if target's node given access to current root BEFORE: - check if current root is super root - check if it's owned by the current root - check if target's node given access to current root - check if target's root given access to current root
235c9cc
to
58ab554
Compare
0f4e5e4
to
337e394
Compare
337e394
to
2d7b101
Compare
4eca7b4
to
03dbf72
Compare
4895d09
to
d75fc1d
Compare
return super().setUp() | ||
|
||
def test_import_basic_python(self) -> None: | ||
"""Test basic self loading.""" | ||
Jac.context().init_memory(base_path=self.fixture_abs_path(__file__)) | ||
ExecutionContext.create(base_path=self.fixture_abs_path(__file__)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should get the context from the plugin, right? just like the rest of the codebase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will update
return Jac.context().mem | ||
def current_context() -> ExecutionContext: | ||
"""Get current execution context.""" | ||
return ExecutionContext.get() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the new_context vs current_context is to handle the difference between jac run via CLI and API request handling? Do we need to handle it explicitly like this? I think the ContextVar is either per thread or per task (in the case of asyncio). so it will automatically be a new one for new request.
From chatgpt
Yes, `ContextVar` in Python is designed to maintain context-specific data, and it behaves differently depending on the concurrency model.
- **Thread-based concurrency:** In a traditional thread-based model, each thread has its own separate context, so `ContextVar` instances are indeed per-thread. Each thread will have its own independent value for a `ContextVar`, meaning changes to a `ContextVar` in one thread will not affect the value in another thread.
- **Async concurrency (using `asyncio`):** In an asynchronous model, `ContextVar` is per-task. Each `asyncio` task has its own context, and changes to a `ContextVar` in one task will not affect the value in another task.
This design ensures that `ContextVar` provides isolation across threads and tasks, making it useful for managing context-specific data in concurrent programming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my assumptions is context should only be created per request/task and can only be read or update afterwards
in jaclang perspective,
new_context should be triggered only on cli
while current_context is for language level
CHANGES:
!!! IMPORTANT !!!
!!! --------- !!!
!!! TO FOLLOW !!!
!!! --------- !!!