-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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(frontend): do not use backend url in <img> #2424
Conversation
BACKEND_URL.length - 1, | ||
)}${BASE_URL_API}files/profile_pictures/${userData?.profile_image}` ?? | ||
profileCircle; | ||
`${BASE_URL_API}files/profile_pictures/${ |
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.
Hi @nicoloboschi,
I appreciate the refactor that removes the code from the src tag in the HTML.
I just wanted to provide some context and ask a question regarding the variable BACKEND_URL.
This variable was created because Langflow's frontend and backend do not always run on the same server URL. Hence, BACKEND_URL was introduced to handle this scenario.
If we revert to using BASE_URL_API, do you think it might break this setup? What are your thoughts?
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.
Hey @Cristhianzl
the scenario is exactly that one, backend and frontend as separate service.
Removing the host will make the browser to reuse the same base host of the current webpage.
since this is an API call the correct flow is:
- call <frontend_service>/api/....
- The frontend redirects the call to the backend (as it does for other calls)
I've verified the fix with different ports/host for backend and frontend and it works well.
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.
Makes sence..
Awesome, I will trust in you in that case hahah
Thanks for your explanation and reply!
LGTM
Cannot auto-update because of conflicts. |
1 similar comment
Cannot auto-update because of conflicts. |
Cannot auto-update because of conflicts. |
Merge commits are not allowed on this repository
* fronted: do not use backend url in <img> * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> (cherry picked from commit b44e43a)
There's already a redirect in place for api calls.
This affect the user profile and the file react component in general
Fixes #2408