-
Notifications
You must be signed in to change notification settings - Fork 344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
experimental: tracing panel #3168
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@Light2Dark is attempting to deploy a commit to the marimo Team on Vercel. A member of the Team first needs to authorize it. |
7133e39
to
2b3050c
Compare
@Light2Dark i can't figure out how to contribute back to your repo, but you can cherry-pick this commit: 1f58e53 there are some todo and some not implemented stuff. also refreshing / SessionView likely wont work (and likely don't need in the first iteration) |
5e90d14
to
30dc547
Compare
ed5801f
to
11e278c
Compare
|
@@ -1227,6 +1227,22 @@ async def test_temporaries_deleted( | |||
await k.run([ExecutionRequest(er.cell_id, "None")]) | |||
assert f"_cell_{er.cell_id}_x" not in k.globals | |||
|
|||
async def test_has_run_id( |
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'm not sure why this test fails, mocked_kernel.stream.messages
does contain run_id butparse_raw
doesn't return an object with run_id
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.
oh i accidentlly gave you a bug. it's trying to post-init again,
def __post_init__(self) -> None:
+ if self.run_id is not None:
+ return
try:
self.run_id = RUN_ID_CTX.get()
except LookupError:
# Be specific about the exception we're catching
# The context variable hasn't been set yet
# may want to explore why this is happening in the future
self.run_id = None
except Exception as e:
LOGGER.error("Error getting run id: %s", str(e))
self.run_id = None
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.
interesting..oh wait, yes I remember why this doesn't work. If you refresh the page, empty / previous (?) runs are maintained
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.
okiee have a fix
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.
thanks for all the suggestions & backend help! I think dark theme was a good addition too
:)) just the backend test left. |
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.
sick, this is amazing! ⚡
theme={props.theme === "dark" ? "dark" : undefined} | ||
width={props.width} | ||
height={props.height} | ||
signalListeners={props.signalListeners} |
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 think actions={false}
would look better
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.
AMAZING WORK!
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.10.7-dev26 |
## 📝 Summary <!-- Provide a concise summary of what this pull request is addressing. If this PR fixes any issues, list them here by number (e.g., Fixes #123). --> Fixes #2898. A v1 tracing panel for observability efforts https://github.com/user-attachments/assets/1c042e0b-2f70-4ec0-ad63-a0c59d5a89a4 ## 🔍 Description of Changes <!-- Detail the specific changes made in this pull request. Explain the problem addressed and how it was resolved. If applicable, provide before and after comparisons, screenshots, or any relevant details to help reviewers understand the changes easily. --> - view cell runs, status, code snippet and timestamps - hover sync between react components and vega chart - supports dark theme - each run now has a run_id context from the backend - note: the timestamps here should be 'more accurate' than the frontend timers since they come from code execution in the backend. Hence, there can also be a small discrepancy btwn them. ## 📋 Checklist - [X] I have read the [contributor guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md). - [X] For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on [Discord](https://marimo.io/discord?ref=pr), or the community [discussions](https://github.com/marimo-team/marimo/discussions) (Please provide a link if applicable). - [X] I have added tests for the changes made. - [X] I have run the code and verified that it works as expected. ## 📜 Reviewers <!-- Tag potential reviewers from the community or maintainers who might be interested in reviewing this pull request. Your PR will be reviewed more quickly if you can figure out the right person to tag with @ --> @akshayka OR @mscolnick --------- Co-authored-by: Myles Scolnick <myles@marimo.io>
📝 Summary
Fixes #2898. A v1 tracing panel for observability efforts
videoo.mp4
🔍 Description of Changes
📋 Checklist
📜 Reviewers
@akshayka OR @mscolnick