hm, a resource container is supposed to be used with GET requests in https://cs.chromium.org/chromium/infra/luci/appengine/swarming/handlers_endpoints.py?q=endpoint+file:%5Einfra/luci/appengine/swarming/&sq=package:chromium&l=320

docs: https://cloud.google.com/appengine/docs/python/endpoints/create_api#using_resourcecontainer_for_path_or_querystring_arguments

but looks like Cloud Endpoints does not mind a message in GET requests. I can change the adapter to accept it too

cloud endpoints silently ignores query string parameter if this value is already present in request body. gross

cloud endpoints silently ignores URL path parameter if it is present in request body

curl -X PUT https://cr-buildbucket-dev.appspot.com/_ah/api/buildbucket/v1/builds/9000428170515905216/retry?tags=x:y --data '{"id":1}' -H "Content-Type: applica
tion/json"
{
 "error": {
  "message": "Build 1 not found",
  "reason": "BUILD_NOT_FOUND"
 },
 "kind": "buildbucket#resourcesItem",
 "etag": "\"c-RgmKx2yA5Oax82ub_lSFN0Vss/4KLd_EpgywIfdgooqK_APWoCCqU\""
}

grooooooooss

I am hesitant to implement these bugs in the adapter. I'd rather fix swarming to use resource container for GET request handlers. M-A, thoughts?

You mean put everything in the URL path? That means creating a new API version.

no need for breaking changes. If you use ResourceContainer with same fields instead of subclass of message.Message, the adapter will start working. Also that's the recommended practice for query string parameters (which means, for all GET request handlers), see docs list above.

however, it would involve code duplication: you reuse swarming_rpcs.TaskRequest and defining a resource container with same fields would not be nice.

i am ok supporting a message.Message subclass only for GET requests. This will fix this particular problem and won't introduce ambiguity that Cloud Endpoints (because there is no request body). I will do it tomorrow.

https://codereview.chromium.org/2370413002

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/618c81d1957a49e75b6f351bff3f7ac325bf51eb

commit 618c81d1957a49e75b6f351bff3f7ac325bf51eb
Author: nodir <nodir@chromium.org>
Date: Wed Sep 28 22:58:08 2016

endpoints_webapp2: honor GET requests with messages

Add support for GET request handlers that accept a message instead of a
resource container.

R=maruel@chromium.org
BUG= 650245 

Review-Url: https://codereview.chromium.org/2370413002

[modify] https://crrev.com/618c81d1957a49e75b6f351bff3f7ac325bf51eb/appengine/components/components/endpoints_webapp2.py
[add] https://crrev.com/618c81d1957a49e75b6f351bff3f7ac325bf51eb/appengine/components/components/endpoints_webapp2_test.py

the botlist and tasklist apis seem to now respect query params. 

However, /bot/[id]/tasks and /bot/id/events don't seem to respect their query params, neither fields= nor limit=

yes, fields parameter is not supported by https://cs.chromium.org/chromium/infra/luci/appengine/components/components/endpoints_webapp2.py ATM, see below

limit works though:
curl https://chromium-swarm.appspot.com/_ah/api/swarming/v1/bot/build1-b4/events?limit=1 -H "Authorization: Bearer `authutil token`"
returns one event

I am closing this bug because proxied swarming API respects query params. The fields parameter is special, implemented inside google api guts. I considered supporting it, but the syntax of fields is not publicly documented and quite involved, e.g. it includes attribute filters.

If `fields` parameter is a big deal, consider filing a separate bug.

limit does not work indeed, my example has _ah and i meant to test without it

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/98093011fef28b43e0596777525aae707198eaf4

commit 98093011fef28b43e0596777525aae707198eaf4
Author: nodir <nodir@chromium.org>
Date: Thu Oct 27 17:19:21 2016

endpoints_webapp2: fix parsing params in GET request

If a GET handler uses a resource container, the adaptor did not parse
fields defined in the message. This was a bug in
https://codereview.chromium.org/2370413002.

R=maruel@chromium.org, kjlubick@chromium.org
BUG= 650245 

Review-Url: https://codereview.chromium.org/2456663004

[modify] https://crrev.com/98093011fef28b43e0596777525aae707198eaf4/appengine/components/components/endpoints_webapp2.py
[modify] https://crrev.com/98093011fef28b43e0596777525aae707198eaf4/appengine/components/components/endpoints_webapp2_test.py