Skip to content

Peer Instruction Migration to FastAPI#1209

Open
sethbern wants to merge 16 commits into
RunestoneInteractive:mainfrom
sethbern:peer-fastapi
Open

Peer Instruction Migration to FastAPI#1209
sethbern wants to merge 16 commits into
RunestoneInteractive:mainfrom
sethbern:peer-fastapi

Conversation

@sethbern
Copy link
Copy Markdown
Contributor

@sethbern sethbern commented May 7, 2026

Hey @bnmnetp, I have done a first pass at moving peer instruction over to FastAPI. For now I am focusing on sync PI to get things working end-to-end before touching async.

Sync PI routes are in, Jinja2 templates ported from the old web2py views, and nav links fixed across _base.html, menu.html, and the Sphinx book theme so everything points to /assignment/peer/.... Async stubs are there but not wired up yet. I've tested locally and seems to be working ok.

A couple things I wanted your eyes on before I keep going:

  1. LTI score sending, I am not super familiar with this part of the codebase so I followed the existing pattern. Does the approach look right?
  2. Overall structure good to go before I move on to async or is there anything else that seems wrong?

@bnmnetp
Copy link
Copy Markdown
Member

bnmnetp commented May 7, 2026

Thanks @sethbern . It looks like you found and cleaned up a number of things in my previous work on porting. The overall structure looks good so keep going I say.

I'll take a closer look at the LTI stuff. I'm just back from two weeks of travel, so I've got a number of other things to take care of before I'm fully back at my desk.

@bnmnetp
Copy link
Copy Markdown
Member

bnmnetp commented May 11, 2026

If you could install black and run it to reformat peer.py so it passes the lint tests that would be great.

@sethbern
Copy link
Copy Markdown
Contributor Author

@bnmnetp @barbarer The web2py peer instructor dashboard has a nav bar with tabs for Student Progress, Admin, Grading, Assignments, Practice, and Help/Documentation. Should this nav bar be included in the new FastAPI peer interface at all? And if so, for the tabs that don't have FastAPI routes yet (Student Progress, Practice, Help/Documentation), should I create them, or are they not worth porting?
image

@bnmnetp
Copy link
Copy Markdown
Member

bnmnetp commented May 14, 2026

No, you can leave the nav bar tabs out of the new FastAPI interface.

@sethbern
Copy link
Copy Markdown
Contributor Author

Hey @bnmnetp, I've migrated both synchronous and asynchronous peer instruction to FastAPI and tested locally. I think this is ready for review.

@sethbern sethbern marked this pull request as ready for review May 20, 2026 14:02
@sethbern sethbern requested a review from bnmnetp as a code owner May 20, 2026 14:02
@bnmnetp
Copy link
Copy Markdown
Member

bnmnetp commented May 20, 2026

Great, I'll take a look. The crud_tests are passing locally for me, so I'm curious if you've looked into why they are failing for this PR?

I'll plan to merge soon so that we can run this in parallel with the old until we have tested it a bit more in the wild.

@sethbern
Copy link
Copy Markdown
Contributor Author

I did look into it but couldn't fully figure it out. It seems like CI might be picking up an old version of rsptx.db.crud but I'm not totally sure.

@bnmnetp
Copy link
Copy Markdown
Member

bnmnetp commented May 20, 2026

have you fetched origin/main and rebased?

@bnmnetp
Copy link
Copy Markdown
Member

bnmnetp commented May 20, 2026

CI creates a whole new virtual machine and environment each time it runs so there are no caching issues possible.

@sethbern
Copy link
Copy Markdown
Contributor Author

I just rebased and it is still not passing. I can dig into it a bit more to see whats causing the issue

@bnmnetp
Copy link
Copy Markdown
Member

bnmnetp commented May 20, 2026

I'm sure you know this, but you can run the tests locally too.

@bnmnetp
Copy link
Copy Markdown
Member

bnmnetp commented May 20, 2026

Yikes what happened? This PR now has 423 files changed!!??

@sethbern
Copy link
Copy Markdown
Contributor Author

I accidentally rebased onto origin/main instead of upstream/main, which is why all those extra commits showed up. When I run the tests locally they're all passing on my end

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.

2 participants