illinois-cs241 / broadway-on-demand

Webapp that facilitates on-demand assignment autograding via Broadway.
Other
11 stars 4 forks source link

Admin api routes #166

Open ananthm0203 opened 7 months ago

ananthm0203 commented 7 months ago

Extended API routes to be able to add/edit assignments, add extensions, and add scheduled grader runs. These require admin permissions and course token authentication. This is originally for the broadway-cli on-demand integration.

nd-0r commented 7 months ago

Hey @ananthm0203, looking at these changes along with the CLI now. Did you get a chance to test the CLI with On-Demand locally? If not, just let me know and I can take care of that. In any case, would you mind git-rm'ing all the __pycache__ files and addressing the comments about the gitignore and the requirements.txt? Thanks!

nd-0r commented 7 months ago

I'm starting to realize some issues. Specifically, what happens when a batched request doesn't succeed?

For reference, here's my understanding of the CLI's workflow so far:

  1. Add the assignment
  2. Add the extensions in one batched API request
  3. Add the scheduled runs for the full roster and extensions in one batched API request

For example, what if the API fails to commit a scheduled run for an extension midway through the CLI's deployment? Should the CLI roll-back the changes by deleting all changes for the assignment? Should the CLI just skip the scheduled run? What if the user wants to retry the same deployment again?

I think, ideally, any progress should roll-back if anything in the deployment fails because the CLI should be completely stateless.

One way I can think of to implement this would be to move the CLI functionality into one super-route in the API.

Another way is to keep the CLI design as-is, but add routes to delete changes in the API and add some cleanup functions in the CLI. This would probably require returning identifiers for the database objects to the CLI. Along this same line of thinking, we could separate the process into adding the assignment+(roster scheduled run) and adding the extensions+(extension scheduled runs); then, we'd only roll back if the assignment process fails and print out the netids of the extensions fail.

Thoughts?

ananthm0203 commented 7 months ago

I'm starting to realize some issues. Specifically, what happens when a batched request doesn't succeed?

For reference, here's my understanding of the CLI's workflow so far:

  1. Add the assignment
  2. Add the extensions in one batched API request
  3. Add the scheduled runs for the full roster and extensions in one batched API request

For example, what if the API fails to commit a scheduled run for an extension midway through the CLI's deployment? Should the CLI roll-back the changes by deleting all changes for the assignment? Should the CLI just skip the scheduled run? What if the user wants to retry the same deployment again?

I think, ideally, any progress should roll-back if anything in the deployment fails because the CLI should be completely stateless.

One way I can think of to implement this would be to move the CLI functionality into one super-route in the API.

I do think the API should support delete functionalities, and while I considered a "super-route" in the API (which would keep stuff stateless), that doesn't seem like very good design, ideally you'd want an API to be modular.

Another way is to keep the CLI design as-is, but add routes to delete changes in the API and add some cleanup functions in the CLI. This would probably require returning identifiers for the database objects to the CLI. Along this same line of thinking, we could separate the process into adding the assignment+(roster scheduled run) and adding the extensions+(extension scheduled runs); then, we'd only roll back if the assignment process fails and print out the netids of the extensions fail.

Thoughts?

I think this is a far more extensible approach, and something I considered, but by then I had already bled a lot of time here, so I wanted to push what I had and add updates to this in the future. The advantage of this approach is you could then use the CLI directly to add "default extensions" (which don't require too much configuration), and that could allow for other people to add extensions directly without knowing the structure of the JSON.

nd-0r commented 7 months ago

Yeah, either way would require some significant redesign. I'm realizing I really just want the course admin to be able to add extensions in the .roster repo and have that automatically be sent to broadway via a webhook. But I agree that this has already been a bit of a time sink. We can talk about this at the infra meeting.