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

How should datasette.client interact with base_url #1026

Closed
simonw opened this issue Oct 15, 2020 · 5 comments
Closed

How should datasette.client interact with base_url #1026

simonw opened this issue Oct 15, 2020 · 5 comments
Labels
Milestone

Comments

@simonw
Copy link
Owner

simonw commented Oct 15, 2020

Refs #1023. If Datasette is running with a base_url setting and a plugin calls e.g. datasette.client.get("/-/plugins.json") what should happen?

@simonw
Copy link
Owner Author

simonw commented Oct 15, 2020

I'm inclined to say that internal requests should ignore base_url - since that seems like the right thing for plugins that need to access default Datasette APIs.

The one catch here is plugins that might want to proxy the current incoming URL for some reason - where that incoming request.path could include the base_url.

Actually those should be fine - because it will have been stripped off earlier:

datasette/datasette/app.py

Lines 963 to 968 in 4f7c0eb

async def route_path(self, scope, receive, send, path):
# Strip off base_url if present before routing
base_url = self.ds.config("base_url")
if base_url != "/" and path.startswith(base_url):
path = "/" + path[len(base_url) :]
request = Request(scope, receive)

@simonw
Copy link
Owner Author

simonw commented Oct 20, 2020

We have a solution for this now: datasette.urls from #1033 can be used by plugins to assemble the correct URLs to pass to .get() and friends.

@simonw simonw closed this as completed Oct 20, 2020
@simonw
Copy link
Owner Author

simonw commented Oct 20, 2020

Reconsidering this: I think the .get() etc methods should automatically add the base_url prefix for you, since these APIs are only intended to make internal calls.

The clincher on this is when I went to add a section to the datasette.client documentation recommending you use datasette.urls.path() for every call to them that you make.

But there's a problem: to handle table name escaping users are likely to want to use datasette.urls.table() anyway, like this:

response = await datasette.client.get(datasette.urls.table("db", "table") + ".json")

This risks adding the base_url prefix twice. Maybe the .table() method could return a string-like object that is marked as already having the base_url prefix added, so the client.get() method knows not to add it again.

@simonw simonw reopened this Oct 20, 2020
@simonw
Copy link
Owner Author

simonw commented Oct 20, 2020

That datasette.urls.table("db", "table") + ".json" example is bad because if the table name contains a . it should be ?_format=json instead.

Maybe .table() should have a format="json" option that knows how to do this.

@simonw
Copy link
Owner Author

simonw commented Oct 31, 2020

#1041 can also benefit from the string subclass that shows that base_url has been added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant