Issues 1633 Add frontend_path to config to support running multiple reflex apps off the same domain, and 1583 Show the correct info on where the site is being served. #1724

nevdelap · 2023-08-31T05:33:01Z

All Submissions:

Have you followed the guidelines stated in CONTRIBUTING.md file?
Have you checked to ensure there aren't any other open Pull Requests for the desired changed.

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

New Feature Submission:

Does your submission pass the tests?
Have you linted your code locally prior to submission?

I've run this, all good, except for some pre-existing issues that are in main complaining about test_config.py.

home 15:27:48 (0) ~> cw reflex_check
#!/bin/bash
set -eufo pipefail

readonly ESC=$'\033'"["
readonly BOLD="${ESC}1m"
readonly RESET="${ESC}0m"

echo -e "\n${BOLD}Testing.$RESET"
poetry run pytest tests --cov --no-cov-on-fail --cov-report=

echo -e "\n${BOLD}Linting.$RESET"
poetry run ruff check --fix .
poetry run pyright reflex tests
find reflex tests -name "*.py" -not -path reflex/reflex.py | xargs poetry run darglint

echo -e "\n${BOLD}Formatting.$RESET"
poetry run black reflex tests

echo -e "\n${BOLD}All good.$RESET"

Changes To Core Features:

Have you added an explanation of what your changes do and why you'd like us to include them? In the issues.
Have you written new tests for your core changes, as applicable?
Have you successfully ran tests with your changes locally?

Closes #1633 and #1583.

Update to 0.2.5.

- Tests. - And sorted config in next.config.js template.

Update to latest main.

Update to 0.2.6.

Update to latest.

Update to 0.2.6.

…x-dev#1583)

nevdelap · 2023-08-31T06:46:54Z

After merging and resolving the conflict I've retested locally for the thing that needed resolving, and for the things the changes do, and I've rerun the tests and everything, and it's all good. I'm redeploying to my own sites on DigitalOcean.

nevdelap · 2023-08-31T08:01:33Z

Had to discover that I needed to change event to _event in my backend nginx config, then it all worked like a bought one.

masenf

This looks really great! Just a couple of comments, and will discuss with the team about the walrus operator.

masenf · 2023-08-31T11:02:05Z

reflex/utils/exec.py

@@ -36,9 +37,13 @@ def run_process_and_launch_url(run_command: list[str]):
    )

    for line in processes.stream_logs("Starting frontend", process):
-        if "ready started server on" in line:
-            url = line.split("url: ")[-1].strip()
+        if match := re.search("ready started server on ([0-9.:]+)", line):


the walrus was introduced in python 3.8, but reflex still claims to support 3.7 although we do not test with it... this would certainly require us to drop 3.7 support in the main package.

The team has been discussing dropping 3.7 for at least a month, but we haven't made a solid decision yet. Although 3.7 went end of life 2 months ago (2023-06-27), there are still some supported linux distributions that might be shipping it.

i'm not asking you do change the code here...yet, but will discuss with the team and see if the time has come to drop 3.7 support. (Personally, I'm of the mind, that if we don't test it in CI, then we don't really support it).

There are things you cannot do without the :=, but since this isn't one of them, happy to change it.

masenf · 2023-08-31T11:12:07Z

reflex/utils/exec.py

-        if "ready started server on" in line:
-            url = line.split("url: ")[-1].strip()
+        if match := re.search("ready started server on ([0-9.:]+)", line):
+            url = match.group(1)


Suggested change

url = match.group(1)

url = f"http://{match.group(1)}"

masenf · 2023-08-31T11:13:04Z

reflex/utils/exec.py

+        if match := re.search("ready started server on ([0-9.:]+)", line):
+            url = match.group(1)
+            if get_config().frontend_path != "":
+                url += get_config().frontend_path


I'd prefer to see urllib.parse.urljoin here, to ensure that leading and trailing slashes are squashed.

Suggested change

url += get_config().frontend_path

url = urljoin(url, get_config().frontend_path)

masenf · 2023-08-31T11:14:23Z

reflex/utils/exec.py

            console.print(f"App running at: [bold green]{url}")
+        else:
+            console.debug(line)


i think process.stream_logs already logs each line at debug level.

…ine.

nevdelap · 2023-09-01T00:17:03Z

@masenf I reran the checks and got failures that I think are caused by fed75ea, error: "NoRenderImportVar" is unknown import symbol.

(I typed that two hours ago and didn't press Enter. 🤪)

nevdelap · 2023-09-01T00:28:43Z

Actually, it looks like it is caused by 63b5fbd because the pre-commit wasn't run on it?

masenf · 2023-09-01T00:47:15Z

Yeah that should be fixed now, if you remerge from main

nevdelap · 2023-09-01T20:47:22Z

Hi @masenf. I hadn't run poetry run pytest integration since it's not mentioned in CONTRIBUTING.md, but I've just run it and it worked.

masenf

sorry the CI was flaky, looks good!

nevdelap and others added 12 commits August 22, 2023 15:50

Merge pull request #1 from reflex-dev/main

703ec72

Update to 0.2.5.

Support setting Next.js basePath in Reflex config. (reflex-dev#1633)

2e3ef76

- Tests. - And sorted config in next.config.js template.

Merge pull request #2 from reflex-dev/main

59de2b9

Update to latest main.

Merge pull request #4 from reflex-dev/main

8ca4887

Update to latest main.

Merge pull request #6 from reflex-dev/main

a6e890a

Update to 0.2.6.

Merge pull request #7 from reflex-dev/main

fe31ffe

Update to latest.

Merge pull request #8 from reflex-dev/main

05aa3a1

Update to 0.2.6.

Display the correct running at url with basePath if it is set. (refle…

e7c76de

…x-dev#1583)

Update to latest.

cb88913

Formatting, fixed by black.

668c387

Fix indenting in test data.

f733828

Merge branch 'main' into issue1633

62b892b

nevdelap marked this pull request as draft August 31, 2023 06:46

nevdelap marked this pull request as ready for review August 31, 2023 08:01

masenf requested changes Aug 31, 2023

View reviewed changes

nevdelap added 3 commits September 1, 2023 07:18

Fixed that conflict resolution shouldnt have included console.debug l…

f5ee378

…ine.

Rmove use of :=. Add http:// to url. Use urljoin to build url.

b01ff76

Update to latest.

fccb66d

nevdelap requested a review from masenf August 31, 2023 21:37

masenf approved these changes Aug 31, 2023

View reviewed changes

Update to latest.

d427f15

masenf approved these changes Sep 1, 2023

View reviewed changes

masenf merged commit 41e97bb into reflex-dev:main Sep 2, 2023
34 checks passed

nevdelap deleted the issue1633 branch September 2, 2023 13:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issues 1633 Add frontend_path to config to support running multiple reflex apps off the same domain, and 1583 Show the correct info on where the site is being served. #1724

Issues 1633 Add frontend_path to config to support running multiple reflex apps off the same domain, and 1583 Show the correct info on where the site is being served. #1724

nevdelap commented Aug 31, 2023

nevdelap commented Aug 31, 2023

nevdelap commented Aug 31, 2023 •

edited

Loading

masenf left a comment

masenf Aug 31, 2023

nevdelap Aug 31, 2023

masenf Aug 31, 2023

nevdelap Aug 31, 2023

masenf Aug 31, 2023

nevdelap Aug 31, 2023

masenf Aug 31, 2023

nevdelap Aug 31, 2023

nevdelap commented Sep 1, 2023 •

edited

Loading

nevdelap commented Sep 1, 2023 •

edited

Loading

masenf commented Sep 1, 2023

nevdelap commented Sep 1, 2023

masenf left a comment

	url += get_config().frontend_path
	url = urljoin(url, get_config().frontend_path)

Issues 1633 Add frontend_path to config to support running multiple reflex apps off the same domain, and 1583 Show the correct info on where the site is being served. #1724

Issues 1633 Add frontend_path to config to support running multiple reflex apps off the same domain, and 1583 Show the correct info on where the site is being served. #1724

Conversation

nevdelap commented Aug 31, 2023

All Submissions:

Type of change

New Feature Submission:

Changes To Core Features:

nevdelap commented Aug 31, 2023

nevdelap commented Aug 31, 2023 • edited Loading

masenf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nevdelap commented Sep 1, 2023 • edited Loading

nevdelap commented Sep 1, 2023 • edited Loading

masenf commented Sep 1, 2023

nevdelap commented Sep 1, 2023

masenf left a comment

Choose a reason for hiding this comment

nevdelap commented Aug 31, 2023 •

edited

Loading

nevdelap commented Sep 1, 2023 •

edited

Loading

nevdelap commented Sep 1, 2023 •

edited

Loading