Skip to content

Initial fastapi structure.#94

Merged
marcosfrenkel merged 7 commits into
masterfrom
initial_fastapi_structure
Jun 30, 2025
Merged

Initial fastapi structure.#94
marcosfrenkel merged 7 commits into
masterfrom
initial_fastapi_structure

Conversation

@marcosfrenkel

Copy link
Copy Markdown
Collaborator

We started from a more bare-bones approach based on the webapp branch. The goal is to move fast an get a real chsh measurement. The plan is to make it more robust and better practices as needed in future PRs.

Comment thread src/pqnstack/app/main.py Outdated
app = FastAPI()

# Constants
HTTP_OK = 200

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's use the existing definitions https://fastapi.tiangolo.com/tutorial/response-status-code/#shortcut-to-remember-the-names

Suggested change
HTTP_OK = 200
from fastapi.status import HTTP_200_OK

Comment thread src/pqnstack/app/main.py Outdated
for i in range(2):
for perp in [False, True]:
r = await http_client.get(f"http://{other_node_address}/chsh/set_basis?i={i}&perp={perp}")
if r.status_code != HTTP_OK:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if r.status_code != HTTP_OK:
TODO: Handle other status codes
if r.status_code != HTTP_200_OK:

Comment thread pyproject.toml Outdated
Comment on lines +19 to +20
"fastapi[standard]>=0.115.14",
"httpx>=0.28.1",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there any case where someone might want to use this package without the api? If so, we should put these dependencies in an optional "webapp" dependency group so they aren't forced to download code they won't use. https://docs.astral.sh/uv/concepts/projects/dependencies/#optional-dependencies

Comment thread src/pqnstack/app/main.py Outdated
Comment on lines +69 to +73
@app.get("/chsh/set_basis")
async def request_basis(i: int, *, perp: bool) -> bool:
angle = state.chsh_basis[i] + 90 * perp
logger.info("moving waveplate", extra={"angle": angle})
return True

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

State is changing, so this should be a POST request. I'm also thinking ahead to possibly having other methods for setting the basis.

Suggested change
@app.get("/chsh/set_basis")
async def request_basis(i: int, *, perp: bool) -> bool:
angle = state.chsh_basis[i] + 90 * perp
logger.info("moving waveplate", extra={"angle": angle})
return True
@app.post("/chsh/request-angle-by-basis")
async def request_angle_by_basis(id:int, *, perp: bool = False) -> bool:
angle = state.chsh_basis[id] + 90 * perp
logger.info("moving waveplate", extra={"angle": angle})
return True

Comment thread src/pqnstack/app/main.py Outdated
marcosfrenkel and others added 2 commits June 30, 2025 11:10
Co-authored-by: Benjamin Nussbaum <50522055+Benjamin-Nussbaum@users.noreply.github.com>

@SoroushHoseini SoroushHoseini left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks good to merge for me.

@marcosfrenkel marcosfrenkel merged commit 6c5ec3c into master Jun 30, 2025
6 checks passed
@marcosfrenkel marcosfrenkel deleted the initial_fastapi_structure branch June 30, 2025 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants