Skip to content
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

External assets #3220

Merged
merged 21 commits into from
May 28, 2024
Merged
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
ce72c31
feat: wip rx.asset
abulvenz May 3, 2024
40bd7ab
fix: Removed test exception. Using contemporary cwd.
abulvenz May 3, 2024
3cb68c3
fix: Adding much logic to provide a nice API for the caller.
abulvenz May 3, 2024
38c127d
fix: cleaning up comments.
abulvenz May 3, 2024
c4929ec
fix: The pipeline made me do this.
abulvenz May 3, 2024
73e5a04
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 6, 2024
3806aec
feat: Review comments. Built in symlinks on non-Windows.
abulvenz May 6, 2024
4b80aa4
fix: Using contemporary python libraries.
abulvenz May 6, 2024
76cf9d7
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 7, 2024
592ef9a
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 15, 2024
8cbd3af
feat: Added test. Moving function to _x namespace.
abulvenz May 15, 2024
2bc36f9
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 15, 2024
381b732
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 16, 2024
69b8037
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 17, 2024
f46c3c8
fix: Fixing typing for python 3.8 in hooks.
abulvenz May 17, 2024
635e701
fix: Fixing typing for python 3.8 in hooks.
abulvenz May 17, 2024
a758e33
fix: Fixing typing for python 3.8 in client_state.
abulvenz May 17, 2024
627c91b
fix: Just saw it was alpha-sorted before.
abulvenz May 17, 2024
c5cc4c1
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 21, 2024
ca09e38
fix: Removing superficial Path constructor and inlining variable.
abulvenz May 21, 2024
9972a35
Merge remote-tracking branch 'upstream/main' into external-assets
abulvenz May 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: Adding much logic to provide a nice API for the caller.
  • Loading branch information
abulvenz committed May 3, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 3cb68c3fbb77963682cb10f3e483268a1f9ca361
18 changes: 11 additions & 7 deletions reflex/app.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
"""The main Reflex app."""

from __future__ import annotations

import asyncio
import concurrent.futures
import contextlib
@@ -82,7 +81,7 @@
from reflex.utils.imports import ImportVar
from pathlib import Path
import shutil
import os
import inspect


# Define custom types.
@@ -1322,9 +1321,10 @@ async def on_ping(self, sid):
# Emit the test event.
await self.emit(str(constants.SocketEvent.PING), "pong", to=sid)

def asset(file: str, name: str, dir: str = ".") -> str:

def asset(relative_filename: str, dir: str = ".") -> str:
"""
Add an asset to the app.
Add an asset to the app.
Place the file next to your including python file.
Copies the file to the app's external assets directory.

@@ -1335,11 +1335,15 @@ def asset(file: str, name: str, dir: str = ".") -> str:
)
```
"""
file = inspect.stack()[1].filename
cwd = Path.cwd()
caller_module = inspect.getmodule(inspect.stack()[1][0]).__name__.replace(".", "/")
dir = caller_module if dir == "." else caller_module + "/" + dir
assets = constants.Dirs.APP_ASSETS
external = constants.Dirs.EXTERNAL_APP_ASSETS
Path(Path(cwd)/ assets / external).mkdir(parents=True, exist_ok=True)
Path(Path(cwd) / assets / external / dir).mkdir(parents=True, exist_ok=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create a variable for Path(Path(cwd) / assets / external / dir) first and use it in the next lines. Also check if the source exists with pathlib's .resolve() method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Lendemor I hope you are fine with using symlinks here, since we are not linking directories. That seems to be the only difference. https://docs.python.org/3/library/pathlib.html#pathlib.Path.symlink_to

@benedikt-bartscher I just tested what happens when src_file is not present. It already complains No such file or directory. OK, or wrap it into a an AssetNotPresent?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abulvenz awesome, if there is already a FileNotFound exception, I think we are fine, no additional wrapping needed

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abulvenz Sorry for delay in review, was on break for a few days.

It's passing windows CI so it should be fine I think.

I'm not sure about adding it inside app.py however, so could you move it to it's own file? Maybe inside reflex/experimental so we'll get some time to test it in real situations before making it official.

We'll also need to add unit tests for it.

Copy link
Contributor Author

@abulvenz abulvenz May 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Lendemor, After v0.5.0 everybody earned a small break 🚀 . I will move it to experimental and try to add a unit test.

shutil.copy2(
Path(file).parent / name, Path(cwd) / assets / external / name
Path(file).parent / relative_filename,
Path(cwd) / assets / external / dir / relative_filename,
)
return "/" + external + "/" + name
return "/" + external + "/" + dir + "/" + relative_filename