Project

Profile

Help

Story #763

closed

As a user, I can search for Pulp things without using the word /search/ in the URL

Added by bmbouter about 9 years ago. Updated about 5 years ago.

Status:
CLOSED - WONTFIX
Priority:
Normal
Assignee:
-
Category:
-
Sprint/Milestone:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Platform Release:
Groomed:
No
Sprint Candidate:
No
Tags:
Pulp 2
Sprint:
Quarter:

Description

The /search/ urls can be replaced with GET and POST parameters sent to URLs we already have. This is a more RESTful approach. You can see all of the urls here: https://github.com/pulp/pulp/blob/master/server/pulp/server/webservices/urls.py

Also if any notes got added to the docs where 'search' was disallowed or code was put in place to disallow in certain places in Pulp (ie: the fix for #761) then those should be removed also.

Actions #1

Updated by bmbouter about 9 years ago

  • Description updated (diff)
Actions #2

Updated by mhrivnak about 9 years ago

As was raised in a recent meeting (sorry I don't recall who brought it up), POST to a REST collection endpoint already has a non-search meaning. There may be ways around that, but suffice it to say, the "right" solution is not entirely clear.

Right now we require POST for search because API users need the ability to provide large documents of search criteria (lots of unit keys, for example). We could try to eliminate that need, but that would be a huge undertaking.

Actions #3

Updated by bmbouter about 9 years ago

mhrivnak wrote:

As was raised in a recent meeting (sorry I don't recall who brought it up), POST to a REST collection endpoint already has a non-search meaning. There may be ways around that, but suffice it to say, the "right" solution is not entirely clear.

Right now we require POST for search because API users need the ability to provide large documents of search criteria (lots of unit keys, for example). We could try to eliminate that need, but that would be a huge undertaking.

POST usually has a create meaning in REST which is one reason it's not RESTful to use POST for search, even to a /search/ URL. GET HTTP requests also have a body just like a POST request; we could support GET for search, allow GET style params like regular webservices, but also support SearchAPI like search expression into the body of the GET request. What do you think about that as an approach?

Actions #4

Updated by rbarlow about 9 years ago

On 03/19/2015 02:52 PM, Pulp wrote:

GET HTTP requests also have a body just like a POST request; we could
support GET for search, allow GET style params like regular webservices,
but also support SearchAPI like search expression into the body of the
GET request. What do you think about that as an approach?

I like this suggestion.

--
Randy Barlow

Actions #5

Updated by mhrivnak about 9 years ago

HTTP 1.1 does not allow a body in a GET request. From RFC 2616:

"A message-body MUST NOT be included in a request if the specification of the request method (section 5.1.1) does not allow sending an entity-body in requests."

The GET method spec does not allow for a body in the request. Some have argued that the above is ambiguous, but it seems pretty conclusive to me.

Also, cachability of GET requests is an important property to preserve. A caching proxy should be able to safely cache responses based on uniqueness of the request-uri.

I actually think it is completely RESTy to have an endpoint that is more of a "controller", that does not do CRUD on a resource, and accepts a POST request to take arbitrary input. I've done a fair amount of reading about RESTful design, and this is the normal way to handle that general use case.

We might find a better place to put the endpoint, or we might be able to remove the need for it in this case. But I don't think using the basic idea of this endpoint is wrong.

Actions #6

Updated by bmbouter about 9 years ago

mhrivnak wrote:

HTTP 1.1 does not allow a body in a GET request. From RFC 2616:

"A message-body MUST NOT be included in a request if the specification of the request method (section 5.1.1) does not allow sending an entity-body in requests."

Good point. Let's not violate the RFC by putting a body on GET requests.

The GET method spec does not allow for a body in the request. Some have argued that the above is ambiguous, but it seems pretty conclusive to me.

Also, cachability of GET requests is an important property to preserve. A caching proxy should be able to safely cache responses based on uniqueness of the request-uri.

I actually think it is completely RESTy to have an endpoint that is more of a "controller", that does not do CRUD on a resource, and accepts a POST request to take arbitrary input. I've done a fair amount of reading about RESTful design, and this is the normal way to handle that general use case.

Given that we need arbitrarily long search queries which will then require a POST I think having a dedicated search API endpoint is probably required.

We might find a better place to put the endpoint, or we might be able to remove the need for it in this case. But I don't think using the basic idea of this endpoint is wrong.

Having a /search/ controller sounds fine, but maybe we could consolidate them? Currently we have 10, but even with so many, we don't provide search capability on many things. With our current design, if you want to make a part of Pulp searchable you have to add a new URL even though they all use the "SearchAPI". What if SearchAPI was adjusted slightly to allow you to search for all things in Pulp in one place. I think adding an identifier so we know what collection to search in would be the adjustment needed. This would make provide one place where users can search for anything, and reduce the effort to make new things searchable over time.

This won't be an amazing improvement, but I do think it is worth the effort to do.

What are thoughts on this type of change?

Actions #7

Updated by rbarlow about 9 years ago

On 03/20/2015 11:58 AM, Pulp wrote:

Having a /search/ controller sounds fine, but maybe we could consolidate
them? Currently we have 10, but even with so many, we don't provide
search capability on many things. With our current design, if you want
to make a part of Pulp searchable you have to add a new URL even though
they all use the "SearchAPI". What if SearchAPI was adjusted slightly to
allow you to search for all things in Pulp in one place. I think adding
an identifier so we know what collection to search in would be the
adjustment needed. This would make provide one place where users can
search for anything, and reduce the effort to make new things searchable
over time.

This won't be an amazing improvement, but I do think it is worth the
effort to do.

What are thoughts on this type of change?

I agree about not violating the RFCs. I almost wrote a reply to Mike
saying "well alright smyers, quoting the RFCs at us again‽" Hahaha.

I like the above suggestion. I think it's pretty much the same as we
have now, but it doesn't collide with the IDs of the objects in the APIs.

Another possibility would be to move the IDs down a level, maybe
something like this:

/users/search/
/users/id/<user_id>/

That way "search" can't be confused with the user_id.

I think your suggestion is every bit as logical as above, and it also
lets the /users/<user_id>/ remain as it was before, which I think is
also a little cleaner. Because of that, I think I like your suggestion a
little more. I just wanted to mention another option I had considered.

--
Randy Barlow

Actions #8

Updated by bmbouter about 5 years ago

  • Status changed from NEW to CLOSED - WONTFIX
Actions #9

Updated by bmbouter about 5 years ago

Pulp 2 is approaching maintenance mode, and this Pulp 2 ticket is not being actively worked on. As such, it is being closed as WONTFIX. Pulp 2 is still accepting contributions though, so if you want to contribute a fix for this ticket, please reopen or comment on it. If you don't have permissions to reopen this ticket, or you want to discuss an issue, please reach out via the developer mailing list.

Actions #10

Updated by bmbouter about 5 years ago

  • Tags Pulp 2 added

Also available in: Atom PDF