server: clean up static assets handling#24550
Conversation
|
Looks like we had a similar idea, #24551 |
lmao, indeed haha 
|
| # Compute relative path from DIST_DIR to each found file. | ||
| # e.g. /path/to/build/tools/ui/dist/_app/immutable/bundle.XXX.js | ||
| # -> _app/immutable/bundle.XXX.js | ||
| foreach(f ${detected_bundle_js}) | ||
| string(REPLACE "${DIST_DIR}/" "" rel "${f}") | ||
| list(APPEND args "${rel}" "${f}") | ||
| endforeach() | ||
| foreach(f ${detected_bundle_css}) | ||
| string(REPLACE "${DIST_DIR}/" "" rel "${f}") | ||
| list(APPEND args "${rel}" "${f}") | ||
| endforeach() | ||
| foreach(f ${detected_workbox}) | ||
| string(REPLACE "${DIST_DIR}/" "" rel "${f}") | ||
| list(APPEND args "${rel}" "${f}") |
There was a problem hiding this comment.
@aldehir for ref, this is part of the reason why I don't want to have nested file structure; the code looks quite overkill IMO
There was a problem hiding this comment.
Yeah that's understandable, although in my PR I simply embed each asset by its path relative to the dist dir and serve it up at the appropriate endpoint. Making the nested file paths irrelevant. Either way, I'll create a new PR and build off of yours.
|
Ok, I applied my changes. Although, if we're going to upload an archive and extract, I don't really see the point of the bundle.css/js renaming / path manipulation. |
|
@allozaur I don't see anything using the |
| auto serve_bundle = [serve_asset_cached](const httplib::Request & req, httplib::Response & res) { | ||
| std::string path = req.path; | ||
| std::string name; | ||
| const char * mime; | ||
| if (path.rfind("/_app/immutable/bundle.", 0) == 0 && path.size() > 22) { | ||
| name = path.substr(1); // strip leading / | ||
| mime = "application/javascript; charset=utf-8"; | ||
| } else if (path.rfind("/_app/immutable/assets/bundle.", 0) == 0 && path.size() > 30) { | ||
| name = path.substr(1); // strip leading / | ||
| mime = "text/css; charset=utf-8"; | ||
| } else { | ||
| res.status = 404; | ||
| return false; | ||
| } | ||
| return serve_asset_cached(name, mime, false)(req, res); |
There was a problem hiding this comment.
@aldehir to reply to your point:
I don't really see the point of the bundle.css/js renaming / path manipulation.
that's true in practice, as the general logic is to serve the dist as-is.
however, we already have this logic in the previous version where we remap whatever hash to the same file. I kinda think that this "relax" logic can be a bit safer, just in case things doesn't matched up then it will continue to work. although I could be wrong.
in either way, we probably need to rethink this in a follow-up PR. for now this one I think it's better to make it bug-for-bug compatible
|
I'm merging this now to fix the release first, let's fix/discuss the rest via follow-up PRs. thanks! |
| - name: Create distribution archive | ||
| run: | | ||
| tar -czf dist.tar.gz -C tools/ui/dist . | ||
| sha256sum dist.tar.gz > dist.tar.gz.sha256 | ||
| mv dist.tar.gz dist.tar.gz.sha256 tools/ui/dist/ |
There was a problem hiding this comment.
@aldehir do you think we should clean up the other files and only preserve dist.tar.gz* ?
ref: https://huggingface.co/buckets/ggml-org/llama-ui/tree/latest?sort=uploadedAt
There was a problem hiding this comment.
Yeah, I was just looking at that. I agree we should clean it up.
Overview
cont #23871
make the handling code as dry as possible
the internal API is also changed:
Changes to post-build
In order to fix problems with dynamic file name (file name with hash - ref: #23871 (comment))
_app/**/*are moved to rootRequirements