rx._x.asset improvements #3624
rx._x.asset improvements #3624
Conversation
benedikt-bartscher
changed the title
|
@benedikt-bartscher I've had a look at the changes and slept one night on it :)
On my option the automatic detection can be removed and then immediately raise an exception when One question: Is it wanted, that missing file presence of local assets should raise when |
reflex/experimental/assets.py
Outdated
| if not dst_file.exists() and ( | ||
| not dst_file.is_symlink() or dst_file.resolve() != src_file_shared.resolve() | ||
| ): | ||
| dst_file.symlink_to(src_file_shared) |
There was a problem hiding this comment.
this makes me a bit nervous because symlinks are not generally enabled or available to unprivileged users on windows.
can we have a fallback path that just copies the files on windows or if this operation fails
There was a problem hiding this comment.
this PR did not introduce the symlinks, they were there before. i can address it in another PR
reflex/experimental/assets.py
Outdated
| """Add an asset to the app. | ||
| def asset( | ||
| path: str, | ||
| shared: bool = False, |
There was a problem hiding this comment.
the shared argument is a little bit confusing to me... is shared=False for referencing files that are already in the app's assets/ directory?
i suppose this is to align the API to work with source-relative assets as well as assets that are already in the correct place? But setting the default to False seems to break how existing callers are already using this function to reference source-relative assets.
There was a problem hiding this comment.
shared=True ist for librarys which want to expose the assets to other python modules/reflex apps. Shared mode expects the files to be relative to the python source file where rx._x.asset is called.
shared=False is for "normal"/"local" assets in reflex. It just returns the serving path and checks if the file really exits.
|
Marked this as draft until re-synced with main. |
There was a problem hiding this comment.
@benedikt-bartscher i updated this PR to move asset out of the experimental namespace with the new API (shared=False default). The existing rx._x.asset has been modified to call the new API with the previous default to avoid breaking any code.
The old _x name will be removed in a few weeks when 0.7.0 is released.
|
@masenf that's awesome, thank you very much. I will rebase our fork and report if everything still works |
Closes #3623