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

Add an endpoint to initialize the GUI #109

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 18 additions & 4 deletions imagedephi/gui.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from pathlib import Path

from fastapi import BackgroundTasks, FastAPI, Form, HTTPException, Request
from fastapi.responses import HTMLResponse, PlainTextResponse
from fastapi.responses import HTMLResponse, PlainTextResponse, RedirectResponse
from fastapi.templating import Jinja2Templates
from jinja2 import FunctionLoader
from starlette.background import BackgroundTask
Expand Down Expand Up @@ -74,11 +74,25 @@ def on_internal_error(request: Request, exc: Exception) -> PlainTextResponse:
)


@app.get("/", response_class=HTMLResponse)
@app.get("/", response_class=RedirectResponse)
def home(request: Request) -> str:
# On Windows, there may be multiple roots, so pick the one that's an ancestor of the CWD
# On Linux, this should typically resolve to "/"
root_directory = Path.cwd().root

# TODO: FastAPI has a bug where a URL object can't be directly returned here
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@brianhelba TODO: report this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm going to file local and upstream issues, then delete this code comment.

return str(
request.url_for("select_directory").include_query_params(
input_directory=str(root_directory), output_directory=str(root_directory)
)
)


@app.get("/select-directory", response_class=HTMLResponse)
def select_directory(
request: Request,
input_directory: Path = Path("/"), # noqa: B008
output_directory: Path = Path("/"), # noqa: B008
input_directory: Path,
output_directory: Path,
):
# TODO: if input_directory is specified but an empty string, it gets instantiated as the CWD
if not input_directory.is_dir():
Expand Down
9 changes: 6 additions & 3 deletions imagedephi/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import webbrowser

import click
from fastapi.datastructures import URL
from hypercorn import Config
from hypercorn.asyncio import serve
import yaml
Expand Down Expand Up @@ -106,9 +107,11 @@ async def announce_ready() -> None:
# To avoid race conditions, ensure that the webserver is
# actually running before launching the browser
await wait_for_port(port)
url = f"http://{host}:{port}/"
click.echo(f"Server is running at {url} .")
webbrowser.open(url)
home_url = app.url_path_for("home").make_absolute_url(
URL(scheme="http", netloc=f"{host}:{port}")
)
click.echo(f"Server is running at {home_url} .")
webbrowser.open(str(home_url))

async with asyncio.TaskGroup() as task_group:
task_group.create_task(announce_ready())
Expand Down
10 changes: 6 additions & 4 deletions tests/test_gui.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ def client() -> TestClient:
return TestClient(app)


def test_gui_select_directory(client: TestClient) -> None:
response = client.get("/")
def test_gui_home(client: TestClient) -> None:
response = client.get(app.url_path_for("home"), follow_redirects=True)

assert response.status_code == 200
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we could put some more specific assertions in here, to ensure it really is the root that's being redirected to. I believe that was the problem raised in #59.

assert "Select Directory" in response.text
Expand All @@ -35,7 +35,8 @@ def test_gui_select_directory_input_not_found(
tmp_path: Path,
) -> None:
response = client.get(
app.url_path_for("select_directory"), params={"input_directory": str(tmp_path / "fake")}
app.url_path_for("select_directory"),
params={"input_directory": str(tmp_path / "fake"), "output_directory": str(tmp_path)},
)

assert response.status_code == 404
Expand All @@ -47,7 +48,8 @@ def test_gui_select_directory_output_not_found(
tmp_path: Path,
) -> None:
response = client.get(
app.url_path_for("select_directory"), params={"output_directory": str(tmp_path / "fake")}
app.url_path_for("select_directory"),
params={"input_directory": str(tmp_path), "output_directory": str(tmp_path / "fake")},
)

assert response.status_code == 404
Expand Down