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

Make route matched pattern groups more consistent #1667

Closed
simonw opened this issue Mar 19, 2022 · 3 comments
Closed

Make route matched pattern groups more consistent #1667

simonw opened this issue Mar 19, 2022 · 3 comments

Comments

@simonw
Copy link
Owner

simonw commented Mar 19, 2022

... highlights how inconsistent the way the capturing works is. Especially as_format which can be None or "" or .json or json or not used at all in the case of TableView.

@pytest.mark.parametrize(
"path,expected_class,expected_matches",
(
("/", "IndexView", {"as_format": ""}),
("/foo", "DatabaseView", {"as_format": None, "db_name": "foo"}),
("/foo.csv", "DatabaseView", {"as_format": ".csv", "db_name": "foo"}),
("/foo.json", "DatabaseView", {"as_format": ".json", "db_name": "foo"}),
("/foo.humbug", "DatabaseView", {"as_format": None, "db_name": "foo.humbug"}),
("/foo/humbug", "TableView", {"db_name": "foo", "table": "humbug"}),
("/foo/humbug.json", "TableView", {"db_name": "foo", "table": "humbug"}),
("/foo/humbug.blah", "TableView", {"db_name": "foo", "table": "humbug"}),
(
"/foo/humbug/1",
"RowView",
{"as_format": None, "db_name": "foo", "pk_path": "1", "table": "humbug"},
),
(
"/foo/humbug/1.json",
"RowView",
{"as_format": ".json", "db_name": "foo", "pk_path": "1", "table": "humbug"},
),
("/-/metadata.json", "JsonDataView", {"as_format": ".json"}),
("/-/metadata", "JsonDataView", {"as_format": ""}),
),
)

Originally posted by @simonw in #1666 (comment)

Part of:

@simonw simonw added this to the Datasette 1.0 milestone Mar 19, 2022
@simonw
Copy link
Owner Author

simonw commented Mar 19, 2022

I called it as_format to avoid clashing with the Python built-in format() function when these things were turned into keyword arguments, but now that they're not I can use format instead.

I think I'm going to go with database, table, format and pks.

@simonw
Copy link
Owner Author

simonw commented Mar 19, 2022

OK, I've made this more consistent - I still need to address the fact that format can be .json or json or not used at all before I close this issue.

("/", "IndexView", {"format": ""}),
("/foo", "DatabaseView", {"format": None, "database": "foo"}),
("/foo.csv", "DatabaseView", {"format": ".csv", "database": "foo"}),
("/foo.json", "DatabaseView", {"format": ".json", "database": "foo"}),
("/foo.humbug", "DatabaseView", {"format": None, "database": "foo.humbug"}),
("/foo/humbug", "TableView", {"database": "foo", "table": "humbug"}),
("/foo/humbug.json", "TableView", {"database": "foo", "table": "humbug"}),
("/foo/humbug.blah", "TableView", {"database": "foo", "table": "humbug"}),
(
"/foo/humbug/1",
"RowView",
{"format": None, "database": "foo", "pks": "1", "table": "humbug"},
),
(
"/foo/humbug/1.json",
"RowView",
{"format": ".json", "database": "foo", "pks": "1", "table": "humbug"},
),
("/-/metadata.json", "JsonDataView", {"format": ".json"}),
("/-/metadata", "JsonDataView", {"format": ""}),
),

@simonw
Copy link
Owner Author

simonw commented Mar 19, 2022

I can now read format from request.url_vars and delete this code entirely:

def get_format(self, request):
# Format is the bit from the path following the ., if one exists
last_path_component = request.path.split("/")[-1]
if "." in last_path_component:
return last_path_component.split(".")[-1]
else:
return None

@simonw simonw closed this as completed in 798f075 Mar 19, 2022
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