search

ontodev / droid

DROID Reminds us that Ordinary Individuals can be Developers
BSD 3-Clause "New" or "Revised" License
4 stars 1 forks source link

Consider default CGI PATH_INFO="/" #123

Closed jamesaoverton closed 2 years ago

jamesaoverton commented 2 years ago

When calling Flask via CGI we are currently sending no PATH_INFO. Flask always responds with a redirect to PATH_INFO="/". Our CGI overhead is significant, and this doubles the time to first page for our Flask apps, and this affects a bunch of tools that I care about.

Arguably default of PATH_INFO="/" is more correct, but we should check the CGI spec. I doubt it would cause any harm.

lmcmicu commented 2 years ago

FYI the CGI Spec says that "a PATH_INFO of "/" represents a single void path segment".

I don't think this is inconsistent with the way we are using it, correct?

jamesaoverton commented 2 years ago

This fix does what I asked. Now I see that I missed something, so I'll add this clarification. No action needed.

For the nanobot use case that I was thinking about here https://droid.ontodev.com/ODD/branches/fix-1/, the Workflow section of the Makefile specifies ./run.py, and when you clicked on that link the old behaviour was actually three hops: https://droid.ontodev.com/ODD/branches/fix-1/views/run.py 302 redirected to https://droid.ontodev.com/ODD/branches/fix-1/views/run.py/ by Flask, then 302 redirected to https://droid.ontodev.com/ODD/branches/fix-1/views/run.py/table by nanobot because the default_table is "table". No wonder it was slow. With the change in #126 we skip the first hop, which is better.

To skip the second hop we would need to add PATH_INFO information to the Workflow, such as ./run.py/table or something. When we designed the CGI feature we decided not to do this: I think it would violate our design principle "DROID is optional" since ./run.py/table is not something you can just read from the Makefile and run on your command line.

So for now, when using nanobot with DROID we will live with the default_table redirect. In the future I expect that we'll add a proper index page for nanobot instead of the default_table reidrect.