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

Consolidate request.raw_args and request.args #774

Closed
simonw opened this issue May 27, 2020 · 8 comments
Closed

Consolidate request.raw_args and request.args #774

simonw opened this issue May 27, 2020 · 8 comments

Comments

@simonw
Copy link
Owner

simonw commented May 27, 2020

request.raw_args is not documented, and I'd like to remove it entirely.
Originally posted by @simonw in #706 (comment)

I use it in a few places in other projects though, so I'll have to fix those first: https://github.com/search?q=user%3Asimonw+raw_args&type=Code

@simonw simonw added this to the Datasette 1.0 milestone May 28, 2020
@simonw
Copy link
Owner Author

simonw commented May 28, 2020

How about moving this functionality to the request object itself?

q = request["q"]  # Raises KeyError if missing, otherwise returns first
q = request.get("q", "default") # Returns first, or optional default or None
facets = request.getlist("_facet")

@simonw
Copy link
Owner Author

simonw commented May 29, 2020

Or change request.args to behave more like request.raw_args - mainly to return a single value when you look things up by key.

It's currently defined like this:

class RequestParameters(dict):
def get(self, name, default=None):
"Return first value in the list, if available"
try:
return super().get(name)[0]
except (KeyError, TypeError):
return default
def getlist(self, name, default=None):
"Return full list"
return super().get(name, default)

@simonw
Copy link
Owner Author

simonw commented May 29, 2020

I think request.args.getlist() should return a list, not None, if the key does not exist.

@simonw simonw changed the title Remove request.raw_args Remove or reconsider request.raw_args May 29, 2020
@simonw simonw changed the title Remove or reconsider request.raw_args Consolidate request.raw_args and request.args May 29, 2020
simonw added a commit that referenced this issue May 29, 2020
Also added some unit tests for request.args
@simonw
Copy link
Owner Author

simonw commented May 29, 2020

I think I want request.args["key"] to return the FIRST item for that key or raise a KeyError if none are found.

Right now it returns the full list.

@simonw
Copy link
Owner Author

simonw commented May 29, 2020

As far as I can tell the only code I've ever written that would break if I made this change is in russian-ira-facebook-ads:

https://github.com/simonw/russian-ira-facebook-ads-datasette/blob/e7106710abdd7bdcae035bedd8bdaba75ae56a12/plugins/target.py#L22

https://github.com/simonw/russian-ira-facebook-ads-datasette/blob/b8a22348c6b315ab94ddba69e8117dfdfd9573dc/plugins/regexp.py#L17

That doesn't work against latest Datasette anyway, so I think I can safely make this change.

@simonw
Copy link
Owner Author

simonw commented May 29, 2020

I'm going to rebuild RequestParameters to no longer subclass dict. I'll keep the following methods:

  • __contains__()
  • __getitem__() (with the new behaviour)
  • keys()
  • iterating iterates keys
  • __len__
  • get
  • getlist

It won't support writing, so it will effectively be immutable after you have constructed it.

@simonw simonw closed this as completed in 81be313 May 29, 2020
@simonw
Copy link
Owner Author

simonw commented May 29, 2020

Updated documentation for RequestParameters: https://datasette.readthedocs.io/en/latest/internals.html#the-requestparameters-class

@simonw
Copy link
Owner Author

simonw commented May 29, 2020

Oh dear... it looks like .raw_args is used in my TIL script, which has been copied by a few people!

https://github.com/search?q=request+raw_args+datasette&type=Code

I'll fix it in mine and file pull requests against other pieces before this code gets released.

simonw added a commit to simonw/museums that referenced this issue May 29, 2020
simonw added a commit to simonw/til that referenced this issue May 29, 2020
simonw added a commit to simonw/til-3 that referenced this issue May 30, 2020
This ensures compatibility with the next release of Datasette.

See simonw/datasette#774
simonw added a commit to simonw/TILs that referenced this issue May 30, 2020
This ensures compatibility with the next release of Datasette.

See simonw/datasette#774
simonw added a commit that referenced this issue Jun 12, 2020
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