-
Notifications
You must be signed in to change notification settings - Fork 153
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
fix: marimo config to follow_symlink in StaticFiles #1327
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
@@ -31,9 +32,14 @@ | |||
# Root directory for static assets | |||
root = os.path.realpath(str(import_files("marimo").joinpath("_static"))) | |||
|
|||
config = UserConfigManager().get_config().get("server", {}) |
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 wasn't sure how to get the MarimoConfig from a top-level context like this. Most call sites seem to read it off an existing object. Is there a better way than re-instantiating a UserConfigManager?
I have read the CLA Document and I hereby sign the CLA |
@@ -31,9 +32,14 @@ | |||
# Root directory for static assets | |||
root = os.path.realpath(str(import_files("marimo").joinpath("_static"))) | |||
|
|||
config = UserConfigManager().get_config().get("server", {}) |
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.
This is the best way given the current setup - and fine as is
@Ubehebe yea the test are snapshot tests - running it once fixes it |
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.
looks great! thanks for contribution @Ubehebe
Hi maintainers, this is a draft of a fix for #1298. It adds a
follow_symlink
boolean flag to Marimo's ServerConfig (defaulting to false), and propagates it to the call site where the StaticFiles for/assets
is constructed. This allows users to set it via their~/.marimo.toml
files like so:I've verified this works by patching this PR into my Bazel repo using the setup described here: #1298 (comment). I also verified this PR passes ruff and mypy.
I've kept this PR simple. This is the first time I've tried to change marimo's codebase, so I could definitely have missed things from the discussion in #1298. Suggestions are welcome.