Fix relative path handling for ragna panel app #280
Conversation
There was a problem hiding this comment.
Thanks Amit for the PR!
This doesn't handles the FastAPI paths yet
I assume you mean "this doesn't handle FastAPI paths automatically yet", correct? I think that is ok. I don't see how the API would be able to detect this in the first place.
I have one question for the JS redirects: it seems we are doing mostly relative redirects, which have the disadvantage that the developer needs to know exactly at which path they currently are. Is there a technical blocker from always doing absolute redirects?
| # so /foo/bar/car/auth becomes /foo/bar/car/ | ||
| >>> redirect_script(remove="auth", append="/") | ||
| """ | ||
| js_script = preformat( |
There was a problem hiding this comment.
IIUC, the only reason we need the preformat with six str.replace is because we want to use str.format later and our original text might already have curly braces inside of it, correct? If so, wouldn't it be easier to rely on re.sub(r"\{\{\w+\}\}") or whatever other delimiters for the substitution you want to use?
There was a problem hiding this comment.
So you mean without using str.format?
|
Does this PR resolve #275 or is there even more to the story? |
Yes, FastAPI would need a root api path, which I have added. If it needs more changes, then it's beyond the scope of this PR.
Mostly, but I am not sure as I said I have not looked at the potential issues with FastAPI redirects. I want to keep the scope of this PR limited to panel redirects only. I'll create a separate PR for potential FastAPI changes if any. I added the root api path config option because it's anyway good to have and was an easy fix to potential redirect issues. |
This doesn't handles the FastAPI paths yet, have just added an optional environment variable:
RAGNA_API_ROOT_PATHfor specifying root path for FastAPI server.